-----Original Message----- From: linux-kernel-owner@vger.kernel.org linux-kernel-owner@vger.kernel.org On Behalf Of Long Li Sent: Tuesday, April 17, 2018 12:17 PM To: Steve French sfrench@samba.org; linux-cifs@vger.kernel.org; samba- technical@lists.samba.org; linux-kernel@vger.kernel.org; linux-rdma@vger.kernel.org Cc: Long Li longli@microsoft.com; stable@vger.kernel.org Subject: [Patch v2 2/6] cifs: Allocate validate negotiation request through kmalloc
From: Long Li longli@microsoft.com
The data buffer allocated on the stack can't be DMA'ed, and hence can't send through RDMA via SMB Direct.
Fix this by allocating the request on the heap in smb3_validate_negotiate.
Fixes: ff1c038addc4f205d5f1ede449426c7d316c0eed "Check SMB3 dialects against downgrade attacks"
Changes in v2: Removed duplicated code on freeing buffers on function exit. (Thanks to Parav Pandit parav@mellanox.com)
Fixed typo in the patch title.
Signed-off-by: Long Li longli@microsoft.com Cc: stable@vger.kernel.org
fs/cifs/smb2pdu.c | 57 ++++++++++++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 26 deletions(-)
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 0f044c4..41625e4 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -729,8 +729,8 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) {
- int rc = 0;
- struct validate_negotiate_info_req vneg_inbuf;
- int ret, rc = -EIO;
- struct validate_negotiate_info_req *pneg_inbuf; struct validate_negotiate_info_rsp *pneg_rsp = NULL; u32 rsplen; u32 inbuflen; /* max of 4 dialects */
@@ -741,6 +741,9 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) if (tcon->ses->server->rdma) return 0; #endif
- pneg_inbuf = kmalloc(sizeof(*pneg_inbuf), GFP_KERNEL);
- if (!pneg_inbuf)
return -ENOMEM;
Immediately after the above new code, there are three if statements that can 'return 0' and never free the pneg_inbuf memory. They should instead set 'rc' appropriately and 'goto' the out_free_inbuf label.
Michael
/* In SMB3.11 preauth integrity supersedes validate negotiate */ if (tcon->ses->server->dialect == SMB311_PROT_ID) @@ -764,53 +767,53 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) if (tcon->ses->session_flags & SMB2_SESSION_FLAG_IS_NULL) cifs_dbg(VFS, "Unexpected null user (anonymous) auth flag sent by server\n");
- vneg_inbuf.Capabilities =
- pneg_inbuf->Capabilities = cpu_to_le32(tcon->ses->server->vals->req_capabilities);
- memcpy(vneg_inbuf.Guid, tcon->ses->server->client_guid,
memcpy(pneg_inbuf->Guid, tcon->ses->server->client_guid, SMB2_CLIENT_GUID_SIZE);
if (tcon->ses->sign)
vneg_inbuf.SecurityMode =
pneg_inbuf->SecurityMode = cpu_to_le16(SMB2_NEGOTIATE_SIGNING_REQUIRED);
vneg_inbuf.SecurityMode =
pneg_inbuf->SecurityMode = cpu_to_le16(SMB2_NEGOTIATE_SIGNING_ENABLED);
vneg_inbuf.SecurityMode = 0;
pneg_inbuf->SecurityMode = 0;if (strcmp(tcon->ses->server->vals->version_string, SMB3ANY_VERSION_STRING) == 0) {
vneg_inbuf.Dialects[0] = cpu_to_le16(SMB30_PROT_ID);vneg_inbuf.Dialects[1] = cpu_to_le16(SMB302_PROT_ID);vneg_inbuf.DialectCount = cpu_to_le16(2);
pneg_inbuf->Dialects[0] = cpu_to_le16(SMB30_PROT_ID);pneg_inbuf->Dialects[1] = cpu_to_le16(SMB302_PROT_ID);pneg_inbuf->DialectCount = cpu_to_le16(2);
vneg_inbuf.Dialects[0] = cpu_to_le16(SMB21_PROT_ID);vneg_inbuf.Dialects[1] = cpu_to_le16(SMB30_PROT_ID);vneg_inbuf.Dialects[2] = cpu_to_le16(SMB302_PROT_ID);vneg_inbuf.DialectCount = cpu_to_le16(3);
pneg_inbuf->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);pneg_inbuf->Dialects[1] = cpu_to_le16(SMB30_PROT_ID);pneg_inbuf->Dialects[2] = cpu_to_le16(SMB302_PROT_ID);pneg_inbuf->DialectCount = cpu_to_le16(3);
vneg_inbuf.Dialects[0] =
pneg_inbuf->Dialects[0] = cpu_to_le16(tcon->ses->server->vals->protocol_id);
vneg_inbuf.DialectCount = cpu_to_le16(1);
pneg_inbuf->DialectCount = cpu_to_le16(1);
- rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
- ret = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID, FSCTL_VALIDATE_NEGOTIATE_INFO, true /* is_fsctl */,
(char *)&vneg_inbuf, sizeof(struct validate_negotiate_info_req),
(char *)pneg_inbuf, sizeof(struct validate_negotiate_info_req),
- if (rc != 0) {
cifs_dbg(VFS, "validate protocol negotiate failed: %d\n", rc);return -EIO;
if (ret != 0) {
cifs_dbg(VFS, "validate protocol negotiate failed: %d\n", ret);goto out_free_inbuf;}
if (rsplen != sizeof(struct validate_negotiate_info_rsp)) {
@@ -820,7 +823,7 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) /* relax check since Mac returns max bufsize allowed on ioctl */ if ((rsplen > CIFSMaxBufSize) || (rsplen < sizeof(struct validate_negotiate_info_rsp)))
goto err_rsp_free;
goto out_free_rsp;}
/* check validate negotiate info response matches what we got earlier */
@@ -838,14 +841,16 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
/* validate negotiate successful */ cifs_dbg(FYI, "validate negotiate info successful\n");
- kfree(pneg_rsp);
- return 0;
- rc = 0;
- goto out_free_rsp;
vneg_out: cifs_dbg(VFS, "protocol revalidation - security settings mismatch\n"); -err_rsp_free: +out_free_rsp: kfree(pneg_rsp);
- return -EIO;
+out_free_inbuf:
- kfree(pneg_inbuf);
- return rc;
}
enum securityEnum
2.7.4