* [PATCH 0/2] CIFS: Adjustments for smb2_ioctl_query_info()
@ 2019-11-05 21:38 ` Markus Elfring
0 siblings, 0 replies; 10+ messages in thread
From: Markus Elfring @ 2019-11-05 21:38 UTC (permalink / raw)
To: linux-cifs, samba-technical, 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] 10+ messages in thread
* [PATCH 1/2] CIFS: Use memdup_user() rather than duplicating its implementation
2019-11-05 21:38 ` Markus Elfring
@ 2019-11-05 21:42 ` Markus Elfring
-1 siblings, 0 replies; 10+ 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 related [flat|nested] 10+ messages in thread* [PATCH 1/2] CIFS: Use memdup_user() rather than duplicating its implementation
@ 2019-11-05 21:42 ` Markus Elfring
0 siblings, 0 replies; 10+ messages in thread
From: Markus Elfring @ 2019-11-05 21:42 UTC (permalink / raw)
To: linux-cifs, 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 related [flat|nested] 10+ messages in thread
* [PATCH 2/2] CIFS: Use common error handling code in smb2_ioctl_query_info()
2019-11-05 21:38 ` Markus Elfring
@ 2019-11-05 21:44 ` Markus Elfring
-1 siblings, 0 replies; 10+ 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 related [flat|nested] 10+ messages in thread* [PATCH 2/2] CIFS: Use common error handling code in smb2_ioctl_query_info()
@ 2019-11-05 21:44 ` Markus Elfring
0 siblings, 0 replies; 10+ messages in thread
From: Markus Elfring @ 2019-11-05 21:44 UTC (permalink / raw)
To: linux-cifs, 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 related [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] CIFS: Adjustments for smb2_ioctl_query_info()
2019-11-05 21:38 ` Markus Elfring
@ 2019-11-05 22:26 ` Steve French
-1 siblings, 0 replies; 10+ 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] 10+ messages in thread
* Re: [PATCH 0/2] CIFS: Adjustments for smb2_ioctl_query_info()
@ 2019-11-05 22:26 ` Steve French
0 siblings, 0 replies; 10+ 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] 10+ messages in thread
* Re: [0/2] CIFS: Adjustments for smb2_ioctl_query_info()
2019-11-05 22:26 ` Steve French
@ 2019-11-06 8:45 ` Markus Elfring
-1 siblings, 0 replies; 10+ 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] 10+ messages in thread