Linux-CIFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] CIFS: Adjustments for smb2_ioctl_query_info()
@ 2019-11-05 21:38 Markus Elfring
  2019-11-05 21:42 ` [PATCH 1/2] CIFS: Use memdup_user() rather than duplicating its implementation Markus Elfring
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Markus Elfring @ 2019-11-05 21:38 UTC (permalink / raw)
  To: linux-cifs, samba-technical, Ronnie Sahlberg, Ronnie Sahlberg
  Cc: LKML, kernel-janitors, Aurelien Aptel

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 5 Nov 2019 22:32:23 +0100

Two update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Use memdup_user() rather than duplicating its implementation
  Use common error handling code in smb2_ioctl_query_info()

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

--
2.24.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] CIFS: Use memdup_user() rather than duplicating its implementation
  2019-11-05 21:38 [PATCH 0/2] CIFS: Adjustments for smb2_ioctl_query_info() Markus Elfring
@ 2019-11-05 21:42 ` Markus Elfring
  2019-11-05 21:44 ` [PATCH 2/2] CIFS: Use common error handling code in smb2_ioctl_query_info() Markus Elfring
  2019-11-05 22:26 ` [PATCH 0/2] CIFS: Adjustments for smb2_ioctl_query_info() Steve French
  2 siblings, 0 replies; 5+ messages in thread
From: Markus Elfring @ 2019-11-05 21:42 UTC (permalink / raw)
  To: linux-cifs, Ronnie Sahlberg, Ronnie Sahlberg
  Cc: LKML, kernel-janitors, Aurelien Aptel

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 5 Nov 2019 21:30:25 +0100

Reuse existing functionality from memdup_user() instead of keeping
duplicate source code.

Generated by: scripts/coccinelle/api/memdup_user.cocci

Fixes: f5b05d622a3e99e6a97a189fe500414be802a05c ("cifs: add IOCTL for QUERY_INFO passthrough to userspace")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/cifs/smb2ops.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 9cbb0ae0e53e..fde2e6d241a8 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -1413,15 +1413,10 @@ smb2_ioctl_query_info(const unsigned int xid,
 	if (smb3_encryption_required(tcon))
 		flags |= CIFS_TRANSFORM_REQ;

-	buffer = kmalloc(qi.output_buffer_length, GFP_KERNEL);
-	if (buffer == NULL)
-		return -ENOMEM;
-
-	if (copy_from_user(buffer, arg + sizeof(struct smb_query_info),
-			   qi.output_buffer_length)) {
-		rc = -EFAULT;
-		goto iqinf_exit;
-	}
+	buffer = memdup_user(arg + sizeof(struct smb_query_info),
+			     qi.output_buffer_length);
+	if (IS_ERR(buffer))
+		return PTR_ERR(buffer);

 	/* Open */
 	memset(&open_iov, 0, sizeof(open_iov));
--
2.24.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] CIFS: Use common error handling code in smb2_ioctl_query_info()
  2019-11-05 21:38 [PATCH 0/2] CIFS: Adjustments for smb2_ioctl_query_info() Markus Elfring
  2019-11-05 21:42 ` [PATCH 1/2] CIFS: Use memdup_user() rather than duplicating its implementation Markus Elfring
@ 2019-11-05 21:44 ` Markus Elfring
  2019-11-05 22:26 ` [PATCH 0/2] CIFS: Adjustments for smb2_ioctl_query_info() Steve French
  2 siblings, 0 replies; 5+ messages in thread
From: Markus Elfring @ 2019-11-05 21:44 UTC (permalink / raw)
  To: linux-cifs, Ronnie Sahlberg, Ronnie Sahlberg
  Cc: LKML, kernel-janitors, Aurelien Aptel

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 5 Nov 2019 22:26:53 +0100

Move the same error code assignments so that such exception handling
can be better reused at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/cifs/smb2ops.c | 45 +++++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index fde2e6d241a8..fde912aabb20 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -1535,35 +1535,32 @@ smb2_ioctl_query_info(const unsigned int xid,
 		if (le32_to_cpu(io_rsp->OutputCount) < qi.input_buffer_length)
 			qi.input_buffer_length = le32_to_cpu(io_rsp->OutputCount);
 		if (qi.input_buffer_length > 0 &&
-		    le32_to_cpu(io_rsp->OutputOffset) + qi.input_buffer_length > rsp_iov[1].iov_len) {
-			rc = -EFAULT;
-			goto iqinf_exit;
-		}
-		if (copy_to_user(&pqi->input_buffer_length, &qi.input_buffer_length,
-				 sizeof(qi.input_buffer_length))) {
-			rc = -EFAULT;
-			goto iqinf_exit;
-		}
+		    le32_to_cpu(io_rsp->OutputOffset) + qi.input_buffer_length
+		    > rsp_iov[1].iov_len)
+			goto e_fault;
+
+		if (copy_to_user(&pqi->input_buffer_length,
+				 &qi.input_buffer_length,
+				 sizeof(qi.input_buffer_length)))
+			goto e_fault;
+
 		if (copy_to_user((void __user *)pqi + sizeof(struct smb_query_info),
 				 (const void *)io_rsp + le32_to_cpu(io_rsp->OutputOffset),
-				 qi.input_buffer_length)) {
-			rc = -EFAULT;
-			goto iqinf_exit;
-		}
+				 qi.input_buffer_length))
+			goto e_fault;
 	} else {
 		pqi = (struct smb_query_info __user *)arg;
 		qi_rsp = (struct smb2_query_info_rsp *)rsp_iov[1].iov_base;
 		if (le32_to_cpu(qi_rsp->OutputBufferLength) < qi.input_buffer_length)
 			qi.input_buffer_length = le32_to_cpu(qi_rsp->OutputBufferLength);
-		if (copy_to_user(&pqi->input_buffer_length, &qi.input_buffer_length,
-				 sizeof(qi.input_buffer_length))) {
-			rc = -EFAULT;
-			goto iqinf_exit;
-		}
-		if (copy_to_user(pqi + 1, qi_rsp->Buffer, qi.input_buffer_length)) {
-			rc = -EFAULT;
-			goto iqinf_exit;
-		}
+		if (copy_to_user(&pqi->input_buffer_length,
+				 &qi.input_buffer_length,
+				 sizeof(qi.input_buffer_length)))
+			goto e_fault;
+
+		if (copy_to_user(pqi + 1, qi_rsp->Buffer,
+				 qi.input_buffer_length))
+			goto e_fault;
 	}

  iqinf_exit:
@@ -1579,6 +1576,10 @@ smb2_ioctl_query_info(const unsigned int xid,
 	free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);
 	free_rsp_buf(resp_buftype[2], rsp_iov[2].iov_base);
 	return rc;
+
+e_fault:
+	rc = -EFAULT;
+	goto iqinf_exit;
 }

 static ssize_t
--
2.24.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] CIFS: Adjustments for smb2_ioctl_query_info()
  2019-11-05 21:38 [PATCH 0/2] CIFS: Adjustments for smb2_ioctl_query_info() Markus Elfring
  2019-11-05 21:42 ` [PATCH 1/2] CIFS: Use memdup_user() rather than duplicating its implementation Markus Elfring
  2019-11-05 21:44 ` [PATCH 2/2] CIFS: Use common error handling code in smb2_ioctl_query_info() Markus Elfring
@ 2019-11-05 22:26 ` Steve French
  2019-11-06  8:45   ` [0/2] " Markus Elfring
  2 siblings, 1 reply; 5+ messages in thread
From: Steve French @ 2019-11-05 22:26 UTC (permalink / raw)
  To: Markus Elfring
  Cc: CIFS, samba-technical, Ronnie Sahlberg, LKML, kernel-janitors,
	Aurelien Aptel

merged into cifs-2.6.git for-next

On Tue, Nov 5, 2019 at 3:38 PM Markus Elfring <Markus.Elfring@web.de> wrote:
>
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 5 Nov 2019 22:32:23 +0100
>
> Two update suggestions were taken into account
> from static source code analysis.
>
> Markus Elfring (2):
>   Use memdup_user() rather than duplicating its implementation
>   Use common error handling code in smb2_ioctl_query_info()
>
>  fs/cifs/smb2ops.c | 58 ++++++++++++++++++++++-------------------------
>  1 file changed, 27 insertions(+), 31 deletions(-)
>
> --
> 2.24.0
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [0/2] CIFS: Adjustments for smb2_ioctl_query_info()
  2019-11-05 22:26 ` [PATCH 0/2] CIFS: Adjustments for smb2_ioctl_query_info() Steve French
@ 2019-11-06  8:45   ` " Markus Elfring
  0 siblings, 0 replies; 5+ messages in thread
From: Markus Elfring @ 2019-11-06  8:45 UTC (permalink / raw)
  To: Steve French, linux-cifs, samba-technical
  Cc: LKML, kernel-janitors, Aurelien Aptel, Ronnie Sahlberg, Steve French

> merged into cifs-2.6.git for-next

Thanks for your positive feedback.


How are the chances to take another look at any more update suggestions
also from my selection of change possibilities?
https://lkml.org/lkml/2017/8/20/104

Regards,
Markus

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-05 21:38 [PATCH 0/2] CIFS: Adjustments for smb2_ioctl_query_info() Markus Elfring
2019-11-05 21:42 ` [PATCH 1/2] CIFS: Use memdup_user() rather than duplicating its implementation Markus Elfring
2019-11-05 21:44 ` [PATCH 2/2] CIFS: Use common error handling code in smb2_ioctl_query_info() Markus Elfring
2019-11-05 22:26 ` [PATCH 0/2] CIFS: Adjustments for smb2_ioctl_query_info() Steve French
2019-11-06  8:45   ` [0/2] " Markus Elfring

Linux-CIFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-cifs/0 linux-cifs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-cifs linux-cifs/ https://lore.kernel.org/linux-cifs \
		linux-cifs@vger.kernel.org
	public-inbox-index linux-cifs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-cifs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git