cifs: Allocate validate negotiation request through kmalloc
Subsystems (3)
PATCH v5 1/1 cifs: Allocate validate negotiation request through kmalloc
From: Long Li <[email protected]>
The data buffer allocated on the stack can't be DMA'ed, ib_dma_map_page will return an invalid DMA address for a buffer on stack. Even worse, this incorrect address can't be detected by ib_dma_mapping_error. Sending data from this address to hardware will not fail, but the remote peer will get junk data.
Fix this by allocating the request on the heap in smb3_validate_negotiate.
Changes in v2:
Removed duplicated code on freeing buffers on function exit. (Thanks to Parav Pandit <[email protected]>) Fixed typo in the patch title.
Changes in v3:
Added "Fixes" to the patch.
Changed several sizeof() to use *pointer in place of struct.
Changes in v4:
Added detailed comments on the failure through RDMA. Allocate request buffer using GPF_NOFS. Fixed possible memory leak.
Changes in v5:
Removed variable ret for checking return value.
Changed to use pneg_inbuf->Dialects[0] to calculate unused space in pneg_inbuf.
--- fs/cifs/smb2pdu.c | 68 +++++++++++++++++++++++++++++++------------------------ 1 file changed, 38 insertions(+), 30 deletions(-)
fs/cifs/smb2pdu.c +38 -30
@@ -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 rc; +struct validate_negotiate_info_req *pneg_inbuf;struct validate_negotiate_info_rsp *pneg_rsp =NULL ;u32 rsplen;u32 inbuflen;/* max of 4 dialects */
@@ -764,63 +764,69 @@ 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 =kmalloc (sizeof (*pneg_inbuf),GFP_NOFS ); +if (!pneg_inbuf) +return -ENOMEM ; + + 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 );else if (global_secflags &CIFSSEC_MAY_SIGN ) - vneg_inbuf.SecurityMode = + pneg_inbuf->SecurityMode =cpu_to_le16 (SMB2_NEGOTIATE_SIGNING_ENABLED );else - 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 );/* structure is big enough for 3 dialects, sending only 2 */ - inbuflen =sizeof (struct validate_negotiate_info_req ) -2 ; + inbuflen =sizeof (*pneg_inbuf) - +sizeof (pneg_inbuf->Dialects[0 ]); }else if (strcmp (tcon->ses->server->vals->version_string,SMBDEFAULT_VERSION_STRING ) ==0 ) { - 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 );/* structure is big enough for 3 dialects */ - inbuflen =sizeof (struct validate_negotiate_info_req ); + inbuflen =sizeof (*pneg_inbuf); }else {/* otherwise specific dialect was requested */ - 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 );/* structure is big enough for 3 dialects, sending only 1 */ - inbuflen =sizeof (struct validate_negotiate_info_req ) -4 ; + inbuflen =sizeof (*pneg_inbuf) - +sizeof (pneg_inbuf->Dialects[0 ]) *2 ; } rc =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_rsp, &rsplen); + (char *)pneg_inbuf, inbuflen, (char **)&pneg_rsp, &rsplen);if (rc !=0 ) {cifs_dbg (VFS ,"validate protocol negotiate failed: %d\n" , rc); -return -EIO ; + rc = -EIO ; + goto out_free_inbuf; } -if (rsplen !=sizeof (struct validate_negotiate_info_rsp )) { + rc = -EIO ; +if (rsplen !=sizeof (*pneg_rsp)) {cifs_dbg (VFS ,"invalid protocol negotiate response size: %d\n" , rsplen);/* relax check since Mac returns max bufsize allowed on ioctl */ -if ((rsplen > CIFSMaxBufSize) - || (rsplen <sizeof (struct validate_negotiate_info_rsp ))) - goto err_rsp_free; +if (rsplen > CIFSMaxBufSize || rsplen <sizeof (*pneg_rsp)) + goto out_free_rsp; }/* check validate negotiate info response matches what we got earlier */
@@ -837,15 +843,17 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) goto vneg_out;/* validate negotiate successful */ + rc =0 ;cifs_dbg (FYI ,"validate negotiate info successful\n" ); -kfree (pneg_rsp); -return 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
Signature · 1 line
2.7.4
PATCH 1/1 cifs: smbd: Enable signing with smbdirect
From: Long Li <[email protected]>
Now signing is supported with RDMA transport.
Remove the code that disabled it.
--- fs/cifs/connect.c | 8 -------- fs/cifs/smb2pdu.c | 5 ----- 2 files changed, 13 deletions(-)
fs/cifs/connect.c +0 -8
@@ -1977,14 +1977,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname, goto cifs_parse_mount_err; } -#ifdef CONFIG_CIFS_SMB_DIRECT -if (vol->rdma && vol->sign) { -cifs_dbg (VFS ,"Currently SMB direct doesn't support signing." -" This is being fixed\n" ); - goto cifs_parse_mount_err; - } -#endif -#ifndef CONFIG_KEYS /* Muliuser mounts require CONFIG_KEYS support */ if (vol->multiuser) {
fs/cifs/smb2pdu.c +0 -5
@@ -737,11 +737,6 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)cifs_dbg (FYI ,"validate negotiate\n" ); -#ifdef CONFIG_CIFS_SMB_DIRECT -if (tcon->ses->server->rdma) -return 0 ; -#endif -/* In SMB3.11 preauth integrity supersedes validate negotiate */ if (tcon->ses->server->dialect ==SMB311_PROT_ID )return 0 ;
Signature · 1 line
2.7.4
Quoted 12 lines
-----Original Message-----
From: [email protected] <[email protected]> On Behalf Of Long Li
Sent: Wednesday, April 25, 2018 2:30 PM
To: Steve French <[email protected]>; [email protected]; samba- [email protected]; [email protected]; linux- [email protected]
Cc: Long Li <[email protected]>
Subject: [PATCH v5] cifs: Allocate validate negotiation request through kmallocFrom: Long Li <[email protected]>
Quoted 8 lines
The data buffer allocated on the stack can't be DMA'ed,
ib_dma_map_pagewill return an invalid DMA address for a buffer on stack. Even worse, this incorrect address can't be detected byib_dma_mapping_error. Sending data from this address to hardware will not fail, but the remote peer will get junk data.Fix this by allocating the request on the heap in
smb3_validate_negotiate.
Looks good.