* [PATCH 1/2] cifs: Don't match port on SMBDirect transport
@ 2019-05-15 21:09 longli
2019-05-15 21:09 ` [PATCH 2/2] cifs: Allocate memory for all iovs in smb2_ioctl longli
2019-05-16 3:00 ` [PATCH 1/2] cifs: Don't match port on SMBDirect transport Steve French
0 siblings, 2 replies; 6+ messages in thread
From: longli @ 2019-05-15 21:09 UTC (permalink / raw)
To: Steve French, linux-cifs, samba-technical, linux-kernel; +Cc: Long Li
From: Long Li <longli@microsoft.com>
SMBDirect manages its own ports in the transport layer, there is no need to
check the port to find a connection.
Signed-off-by: Long Li <longli@microsoft.com>
---
fs/cifs/connect.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 0b3ac8b76d18..8c4121da624e 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2446,6 +2446,10 @@ match_port(struct TCP_Server_Info *server, struct sockaddr *addr)
{
__be16 port, *sport;
+ /* SMBDirect manages its own ports, don't match it here */
+ if (server->rdma)
+ return true;
+
switch (addr->sa_family) {
case AF_INET:
sport = &((struct sockaddr_in *) &server->dstaddr)->sin_port;
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] cifs: Allocate memory for all iovs in smb2_ioctl
2019-05-15 21:09 [PATCH 1/2] cifs: Don't match port on SMBDirect transport longli
@ 2019-05-15 21:09 ` longli
2019-05-15 22:26 ` Pavel Shilovsky
` (2 more replies)
2019-05-16 3:00 ` [PATCH 1/2] cifs: Don't match port on SMBDirect transport Steve French
1 sibling, 3 replies; 6+ messages in thread
From: longli @ 2019-05-15 21:09 UTC (permalink / raw)
To: Steve French, linux-cifs, samba-technical, linux-kernel; +Cc: Long Li
From: Long Li <longli@microsoft.com>
An IOCTL uses up to 2 iovs. The 1st iov is the command itself, the 2nd iov is
optional data for that command. The 1st iov is always allocated on the heap
but the 2nd iov may point to a variable on the stack. This will trigger an
error when passing the 2nd iov for RDMA I/O.
Fix this by allocating a buffer for the 2nd iov.
Signed-off-by: Long Li <longli@microsoft.com>
---
fs/cifs/smb2pdu.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 29f011d8d8e2..710ceb875161 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -2538,11 +2538,25 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct smb_rqst *rqst,
struct kvec *iov = rqst->rq_iov;
unsigned int total_len;
int rc;
+ char *in_data_buf;
rc = smb2_plain_req_init(SMB2_IOCTL, tcon, (void **) &req, &total_len);
if (rc)
return rc;
+ if (indatalen) {
+ /*
+ * indatalen is usually small at a couple of bytes max, so
+ * just allocate through generic pool
+ */
+ in_data_buf = kmalloc(indatalen, GFP_NOFS);
+ if (!in_data_buf) {
+ cifs_small_buf_release(req);
+ return -ENOMEM;
+ }
+ memcpy(in_data_buf, in_data, indatalen);
+ }
+
req->CtlCode = cpu_to_le32(opcode);
req->PersistentFileId = persistent_fid;
req->VolatileFileId = volatile_fid;
@@ -2563,7 +2577,7 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct smb_rqst *rqst,
cpu_to_le32(offsetof(struct smb2_ioctl_req, Buffer));
rqst->rq_nvec = 2;
iov[0].iov_len = total_len - 1;
- iov[1].iov_base = in_data;
+ iov[1].iov_base = in_data_buf;
iov[1].iov_len = indatalen;
} else {
rqst->rq_nvec = 1;
@@ -2605,8 +2619,11 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct smb_rqst *rqst,
void
SMB2_ioctl_free(struct smb_rqst *rqst)
{
- if (rqst && rqst->rq_iov)
+ if (rqst && rqst->rq_iov) {
cifs_small_buf_release(rqst->rq_iov[0].iov_base); /* request */
+ if (rqst->rq_iov[1].iov_len)
+ kfree(rqst->rq_iov[1].iov_base);
+ }
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] cifs: Allocate memory for all iovs in smb2_ioctl
2019-05-15 21:09 ` [PATCH 2/2] cifs: Allocate memory for all iovs in smb2_ioctl longli
@ 2019-05-15 22:26 ` Pavel Shilovsky
2019-05-15 23:11 ` ronnie sahlberg
2019-05-16 2:58 ` Steve French
2 siblings, 0 replies; 6+ messages in thread
From: Pavel Shilovsky @ 2019-05-15 22:26 UTC (permalink / raw)
To: Long Li; +Cc: Steve French, linux-cifs, samba-technical, Kernel Mailing List
ср, 15 мая 2019 г. в 14:10, <longli@linuxonhyperv.com>:
>
> From: Long Li <longli@microsoft.com>
>
> An IOCTL uses up to 2 iovs. The 1st iov is the command itself, the 2nd iov is
> optional data for that command. The 1st iov is always allocated on the heap
> but the 2nd iov may point to a variable on the stack. This will trigger an
> error when passing the 2nd iov for RDMA I/O.
>
> Fix this by allocating a buffer for the 2nd iov.
>
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
> fs/cifs/smb2pdu.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 29f011d8d8e2..710ceb875161 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -2538,11 +2538,25 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct smb_rqst *rqst,
> struct kvec *iov = rqst->rq_iov;
> unsigned int total_len;
> int rc;
> + char *in_data_buf;
>
> rc = smb2_plain_req_init(SMB2_IOCTL, tcon, (void **) &req, &total_len);
> if (rc)
> return rc;
>
> + if (indatalen) {
> + /*
> + * indatalen is usually small at a couple of bytes max, so
> + * just allocate through generic pool
> + */
> + in_data_buf = kmalloc(indatalen, GFP_NOFS);
> + if (!in_data_buf) {
> + cifs_small_buf_release(req);
> + return -ENOMEM;
> + }
> + memcpy(in_data_buf, in_data, indatalen);
> + }
> +
> req->CtlCode = cpu_to_le32(opcode);
> req->PersistentFileId = persistent_fid;
> req->VolatileFileId = volatile_fid;
> @@ -2563,7 +2577,7 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct smb_rqst *rqst,
> cpu_to_le32(offsetof(struct smb2_ioctl_req, Buffer));
> rqst->rq_nvec = 2;
> iov[0].iov_len = total_len - 1;
> - iov[1].iov_base = in_data;
> + iov[1].iov_base = in_data_buf;
> iov[1].iov_len = indatalen;
> } else {
> rqst->rq_nvec = 1;
> @@ -2605,8 +2619,11 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct smb_rqst *rqst,
> void
> SMB2_ioctl_free(struct smb_rqst *rqst)
> {
> - if (rqst && rqst->rq_iov)
> + if (rqst && rqst->rq_iov) {
> cifs_small_buf_release(rqst->rq_iov[0].iov_base); /* request */
> + if (rqst->rq_iov[1].iov_len)
> + kfree(rqst->rq_iov[1].iov_base);
> + }
> }
>
>
> --
> 2.17.1
>
Looks correct.
Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
--
Best regards,
Pavel Shilovsky
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] cifs: Allocate memory for all iovs in smb2_ioctl
2019-05-15 21:09 ` [PATCH 2/2] cifs: Allocate memory for all iovs in smb2_ioctl longli
2019-05-15 22:26 ` Pavel Shilovsky
@ 2019-05-15 23:11 ` ronnie sahlberg
2019-05-16 2:58 ` Steve French
2 siblings, 0 replies; 6+ messages in thread
From: ronnie sahlberg @ 2019-05-15 23:11 UTC (permalink / raw)
To: Long Li; +Cc: Steve French, linux-cifs, samba-technical, LKML
On Thu, May 16, 2019 at 7:10 AM <longli@linuxonhyperv.com> wrote:
>
> From: Long Li <longli@microsoft.com>
>
> An IOCTL uses up to 2 iovs. The 1st iov is the command itself, the 2nd iov is
> optional data for that command. The 1st iov is always allocated on the heap
> but the 2nd iov may point to a variable on the stack. This will trigger an
> error when passing the 2nd iov for RDMA I/O.
>
> Fix this by allocating a buffer for the 2nd iov.
>
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
> fs/cifs/smb2pdu.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 29f011d8d8e2..710ceb875161 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -2538,11 +2538,25 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct smb_rqst *rqst,
> struct kvec *iov = rqst->rq_iov;
> unsigned int total_len;
> int rc;
> + char *in_data_buf;
>
> rc = smb2_plain_req_init(SMB2_IOCTL, tcon, (void **) &req, &total_len);
> if (rc)
> return rc;
>
> + if (indatalen) {
> + /*
> + * indatalen is usually small at a couple of bytes max, so
> + * just allocate through generic pool
> + */
> + in_data_buf = kmalloc(indatalen, GFP_NOFS);
> + if (!in_data_buf) {
> + cifs_small_buf_release(req);
> + return -ENOMEM;
> + }
> + memcpy(in_data_buf, in_data, indatalen);
> + }
> +
> req->CtlCode = cpu_to_le32(opcode);
> req->PersistentFileId = persistent_fid;
> req->VolatileFileId = volatile_fid;
> @@ -2563,7 +2577,7 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct smb_rqst *rqst,
> cpu_to_le32(offsetof(struct smb2_ioctl_req, Buffer));
> rqst->rq_nvec = 2;
> iov[0].iov_len = total_len - 1;
> - iov[1].iov_base = in_data;
> + iov[1].iov_base = in_data_buf;
> iov[1].iov_len = indatalen;
> } else {
> rqst->rq_nvec = 1;
> @@ -2605,8 +2619,11 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct smb_rqst *rqst,
> void
> SMB2_ioctl_free(struct smb_rqst *rqst)
> {
> - if (rqst && rqst->rq_iov)
> + if (rqst && rqst->rq_iov) {
> cifs_small_buf_release(rqst->rq_iov[0].iov_base); /* request */
> + if (rqst->rq_iov[1].iov_len)
> + kfree(rqst->rq_iov[1].iov_base);
You don't need the conditional. kfree(NULL) is safe,.
> + }
> }
>
>
> --
> 2.17.1
>
Reviewed-by: Ronnie sahlberg <lsahlber@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] cifs: Allocate memory for all iovs in smb2_ioctl
2019-05-15 21:09 ` [PATCH 2/2] cifs: Allocate memory for all iovs in smb2_ioctl longli
2019-05-15 22:26 ` Pavel Shilovsky
2019-05-15 23:11 ` ronnie sahlberg
@ 2019-05-16 2:58 ` Steve French
2 siblings, 0 replies; 6+ messages in thread
From: Steve French @ 2019-05-16 2:58 UTC (permalink / raw)
To: Long Li; +Cc: Steve French, CIFS, samba-technical, LKML
merged into cifs-2.6.git for-next
On Wed, May 15, 2019 at 4:10 PM <longli@linuxonhyperv.com> wrote:
>
> From: Long Li <longli@microsoft.com>
>
> An IOCTL uses up to 2 iovs. The 1st iov is the command itself, the 2nd iov is
> optional data for that command. The 1st iov is always allocated on the heap
> but the 2nd iov may point to a variable on the stack. This will trigger an
> error when passing the 2nd iov for RDMA I/O.
>
> Fix this by allocating a buffer for the 2nd iov.
>
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
> fs/cifs/smb2pdu.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 29f011d8d8e2..710ceb875161 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -2538,11 +2538,25 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct smb_rqst *rqst,
> struct kvec *iov = rqst->rq_iov;
> unsigned int total_len;
> int rc;
> + char *in_data_buf;
>
> rc = smb2_plain_req_init(SMB2_IOCTL, tcon, (void **) &req, &total_len);
> if (rc)
> return rc;
>
> + if (indatalen) {
> + /*
> + * indatalen is usually small at a couple of bytes max, so
> + * just allocate through generic pool
> + */
> + in_data_buf = kmalloc(indatalen, GFP_NOFS);
> + if (!in_data_buf) {
> + cifs_small_buf_release(req);
> + return -ENOMEM;
> + }
> + memcpy(in_data_buf, in_data, indatalen);
> + }
> +
> req->CtlCode = cpu_to_le32(opcode);
> req->PersistentFileId = persistent_fid;
> req->VolatileFileId = volatile_fid;
> @@ -2563,7 +2577,7 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct smb_rqst *rqst,
> cpu_to_le32(offsetof(struct smb2_ioctl_req, Buffer));
> rqst->rq_nvec = 2;
> iov[0].iov_len = total_len - 1;
> - iov[1].iov_base = in_data;
> + iov[1].iov_base = in_data_buf;
> iov[1].iov_len = indatalen;
> } else {
> rqst->rq_nvec = 1;
> @@ -2605,8 +2619,11 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct smb_rqst *rqst,
> void
> SMB2_ioctl_free(struct smb_rqst *rqst)
> {
> - if (rqst && rqst->rq_iov)
> + if (rqst && rqst->rq_iov) {
> cifs_small_buf_release(rqst->rq_iov[0].iov_base); /* request */
> + if (rqst->rq_iov[1].iov_len)
> + kfree(rqst->rq_iov[1].iov_base);
> + }
> }
>
>
> --
> 2.17.1
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] cifs: Don't match port on SMBDirect transport
2019-05-15 21:09 [PATCH 1/2] cifs: Don't match port on SMBDirect transport longli
2019-05-15 21:09 ` [PATCH 2/2] cifs: Allocate memory for all iovs in smb2_ioctl longli
@ 2019-05-16 3:00 ` Steve French
1 sibling, 0 replies; 6+ messages in thread
From: Steve French @ 2019-05-16 3:00 UTC (permalink / raw)
To: Long Li; +Cc: Steve French, CIFS, samba-technical, LKML
merged into cifs-2.6.git for-next
On Wed, May 15, 2019 at 4:09 PM <longli@linuxonhyperv.com> wrote:
>
> From: Long Li <longli@microsoft.com>
>
> SMBDirect manages its own ports in the transport layer, there is no need to
> check the port to find a connection.
>
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
> fs/cifs/connect.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 0b3ac8b76d18..8c4121da624e 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2446,6 +2446,10 @@ match_port(struct TCP_Server_Info *server, struct sockaddr *addr)
> {
> __be16 port, *sport;
>
> + /* SMBDirect manages its own ports, don't match it here */
> + if (server->rdma)
> + return true;
> +
> switch (addr->sa_family) {
> case AF_INET:
> sport = &((struct sockaddr_in *) &server->dstaddr)->sin_port;
> --
> 2.17.1
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-05-16 3:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-15 21:09 [PATCH 1/2] cifs: Don't match port on SMBDirect transport longli
2019-05-15 21:09 ` [PATCH 2/2] cifs: Allocate memory for all iovs in smb2_ioctl longli
2019-05-15 22:26 ` Pavel Shilovsky
2019-05-15 23:11 ` ronnie sahlberg
2019-05-16 2:58 ` Steve French
2019-05-16 3:00 ` [PATCH 1/2] cifs: Don't match port on SMBDirect transport Steve French
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).