[PATCH -next 0/2] fix potential wild-ptr-deref in 'smb3_free_compound_rqst'

[PATCH -next 0/2] fix potential wild-ptr-deref in 'smb3_free_compound_rqst'

[Ye Bin]

Ye Bin (2):
  smb3: refactor about 'smb3_init_transform_rq'
  smb3: fix potential wild-ptr-deref in 'smb3_free_compound_rqst'

 fs/cifs/smb2ops.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

-- 
2.31.1

[PATCH -next 1/2] smb3: refactor about 'smb3_init_transform_rq'

[Ye Bin]

Introduce 'orq' and 'nrq' variable for cleanup code. No functional change.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/cifs/smb2ops.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 421be43af425..180f0260cbc4 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -4457,21 +4457,24 @@ smb3_init_transform_rq(struct TCP_Server_Info *server, int num_rqst,
 	int rc = -ENOMEM;
 
 	for (i = 1; i < num_rqst; i++) {
-		npages = old_rq[i - 1].rq_npages;
+		struct smb_rqst *orq = old_rq + i - 1;
+		struct smb_rqst *nrq = new_rq + i;
+
+		npages = orq->rq_npages;
 		pages = kmalloc_array(npages, sizeof(struct page *),
 				      GFP_KERNEL);
 		if (!pages)
 			goto err_free;
 
-		new_rq[i].rq_pages = pages;
-		new_rq[i].rq_npages = npages;
-		new_rq[i].rq_offset = old_rq[i - 1].rq_offset;
-		new_rq[i].rq_pagesz = old_rq[i - 1].rq_pagesz;
-		new_rq[i].rq_tailsz = old_rq[i - 1].rq_tailsz;
-		new_rq[i].rq_iov = old_rq[i - 1].rq_iov;
-		new_rq[i].rq_nvec = old_rq[i - 1].rq_nvec;
+		nrq->rq_pages = pages;
+		nrq->rq_npages = npages;
+		nrq->rq_offset = orq->rq_offset;
+		nrq->rq_pagesz = orq->rq_pagesz;
+		nrq->rq_tailsz = orq->rq_tailsz;
+		nrq->rq_iov = orq->rq_iov;
+		nrq->rq_nvec = orq->rq_nvec;
 
-		orig_len += smb_rqst_len(server, &old_rq[i - 1]);
+		orig_len += smb_rqst_len(server, orq);
 
 		for (j = 0; j < npages; j++) {
 			pages[j] = alloc_page(GFP_KERNEL|__GFP_HIGHMEM);
@@ -4484,14 +4487,14 @@ smb3_init_transform_rq(struct TCP_Server_Info *server, int num_rqst,
 			char *dst, *src;
 			unsigned int offset, len;
 
-			rqst_page_get_length(&new_rq[i], j, &len, &offset);
+			rqst_page_get_length(nrq, j, &len, &offset);
 
-			dst = (char *) kmap(new_rq[i].rq_pages[j]) + offset;
-			src = (char *) kmap(old_rq[i - 1].rq_pages[j]) + offset;
+			dst = (char *) kmap(nrq->rq_pages[j]) + offset;
+			src = (char *) kmap(orq->rq_pages[j]) + offset;
 
 			memcpy(dst, src, len);
-			kunmap(new_rq[i].rq_pages[j]);
-			kunmap(old_rq[i - 1].rq_pages[j]);
+			kunmap(nrq->rq_pages[j]);
+			kunmap(orq->rq_pages[j]);
 		}
 	}
 
-- 
2.31.1

[PATCH -next 2/2] smb3: fix potential wild-ptr-deref in 'smb3_free_compound_rqst'

[Ye Bin]

As 'smb3_init_transform_rq' first set 'new_rq[i].rq_npages', then
allocate page, if allocate page failed will call 'smb3_free_compound_rqst'
to free page. However, there may only allocate part of 'rq_npages' pages
which will lead to wild-ptr-deref when free pages.
To solve above issue just increase 'new_rq[i].rq_npages' after allocate
page success.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/cifs/smb2ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 180f0260cbc4..a1b47ce4ea15 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -4467,7 +4467,6 @@ smb3_init_transform_rq(struct TCP_Server_Info *server, int num_rqst,
 			goto err_free;
 
 		nrq->rq_pages = pages;
-		nrq->rq_npages = npages;
 		nrq->rq_offset = orq->rq_offset;
 		nrq->rq_pagesz = orq->rq_pagesz;
 		nrq->rq_tailsz = orq->rq_tailsz;
@@ -4480,6 +4479,7 @@ smb3_init_transform_rq(struct TCP_Server_Info *server, int num_rqst,
 			pages[j] = alloc_page(GFP_KERNEL|__GFP_HIGHMEM);
 			if (!pages[j])
 				goto err_free;
+			nrq->rq_npages++;
 		}
 
 		/* copy pages form the old */
-- 
2.31.1