| Message ID | 1557954545-17831-2-git-send-email-longli@linuxonhyperv.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] cifs: Don't match port on SMBDirect transport | expand |
ср, 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
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>
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 >
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); + } }