cifs: smbd: Check for iov length on sending the last iov
Subsystems (4)
PATCH v2 1/6 cifs: smbd: Check for iov length on sending the last iov
From: Long Li <[email protected]>
When sending the last iov that breaks into smaller buffers to fit the transfer size, it's necessary to check if this is the last iov.
If this is the latest iov, stop and proceed to send pages.
--- fs/cifs/smbdirect.c | 2 ++ 1 file changed, 2 insertions(+)
fs/cifs/smbdirect.c +2 -0
@@ -2197,6 +2197,8 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst) goto done; } i++; +if (i == rqst->rq_nvec) +break ; } start = i; buflen =0 ;
Signature · 1 line
2.7.4
PATCH v2 2/6 cifs: Allocate validate negotiation request through kmalloc
From: Long Li <[email protected]>
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.
downgrade attacks"
Changes in v2:
Removed duplicated code on freeing buffers on function exit. (Thanks to Parav Pandit <[email protected]>)
Fixed typo in the patch title.
--- fs/cifs/smb2pdu.c | 57 ++++++++++++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 26 deletions(-)
fs/cifs/smb2pdu.c +31 -26
@@ -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 ;/* 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 );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 ; }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 ); }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 ; } - 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 ), (char **)&pneg_rsp, &rsplen); -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
Signature · 1 line
2.7.4
RE: [Patch v2 2/6] cifs: Allocate validate negotiation request through kmalloc
Quoted 15 lines
-----Original Message-----
From: [email protected] [mailto:linux-rdma- [email protected]] On Behalf Of Long Li
Sent: Tuesday, April 17, 2018 2:17 PM
To: Steve French <[email protected]>; [email protected]; samba- [email protected]; [email protected]; linux- [email protected]
Cc: longli <[email protected]>; [email protected]
Subject: [Patch v2 2/6] 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, 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 dialectsagainst downgrade attacks"
Format is:
It should be right above Signed-off signature.
Quoted 57 lines
Changes in v2:
Removed duplicated code on freeing buffers on function exit. (Thanks to Parav Pandit <[email protected]>)Fixed typo in the patch title.
Signed-off-by Long Li <[email protected]>--- fs/cifs/smb2pdu.c | 57 ++++++++++++++++++++++++++++++------------------------(quoted diff) +0
-1- 1 file changed, 31 insertions(+), 26 deletions(-)fs/cifs/smb2pdu.c index 0f044c4..41625e4 renamed from fs/cifs/smb2pdu.c +0
-0100644
fs/cifs/smb2pdu.c +5
-2@@ -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 ;/* In SMB3.11 preauth integrity supersedes validate negotiate */ if (
tcon->ses->server->dialect==SMB311_PROT_ID) @@ -764,53
+767,53 @@ intsmb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)fs/cifs/smb2pdu.c +0
-0if (tcon->ses->session_flags &SMB2_SESSION_FLAG_IS_NULL ) cifs_dbg(VFS, "Unexpected null user (anonymous) auth flag sentby server\n");
fs/cifs/smb2pdu.c +1
-1- vneg_inbuf.Capabilities = + pneg_inbuf->Capabilities = cpu_to_le32(tcon->ses->server->vals-Quoted 1 line
req_capabilities);fs/cifs/smb2pdu.c +2
-2-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 );
fs/cifs/smb2pdu.c int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) · line 785
Quoted 10 lines
fs/cifs/smb2pdu.c +2
-2else 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,
Please check if strncmp() should be used or not.
Quoted 37 lines
fs/cifs/smb2pdu.c +10
-11SMB3ANY_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 ; } 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 ); } 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 ; } - 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),
fs/cifs/smb2pdu.c int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) · line 811
Quoted 1 line
fs/cifs/smb2pdu.c +1
-0+ (char *)pneg_inbuf, sizeof(struct validate_negotiate_info_req),
Use sizeof(*pneg_inbuf)
fs/cifs/smb2pdu.c int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) · line 814
Quoted 6 lines
fs/cifs/smb2pdu.c +1
-3(char **)&pneg_rsp, &rsplen); -if (rc !=0 ) { -cifs_dbg (VFS ,"validate protocol negotiate failed: %d\n" , rc); -return -EIO ; +if (ret !=0 ) {
if (ret) is fine.
fs/cifs/smb2pdu.c int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
Quoted 5 lines
fs/cifs/smb2pdu.c +2
-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
if (rsplen != sizeof(*pneg_rsp))
RE: [Patch v2 2/6] cifs: Allocate validate negotiation request through kmalloc
Quoted 23 lines
Subject: RE: [Patch v2 2/6] cifs: Allocate validate negotiation request through kmalloc
Quoted 18 lines
-----Original Message-----
From: [email protected] [mailto:linux-rdma- [email protected]] On Behalf Of Long Li
Sent: Tuesday, April 17, 2018 2:17 PM
To: Steve French <[email protected]>; [email protected]; samba- [email protected]; [email protected]; linux- [email protected]
Cc: longli <[email protected]>; [email protected]
Subject: [Patch v2 2/6] cifs: Allocate validate negotiation request through kmallocFrom: Long Li <[email protected]>
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.
Quoted 7 lines
Quoted 3 lines
Fixes ff1c038addc4f205d5f1ede449426c7d316c0eed "Check SMB3 dialectsagainst downgrade attacks"
Format is:
Fixes ff1c038addc4 ("Check SMB3 dialects against downgrade attacks") Itshould be right above Signed-off signature.
I will fix up and resend this patch.
How about the rest patches (1, 3-6) in the series? If they don't need any changes, is it okay that I resend this one only?
Quoted 136 lines
Quoted 24 lines
Changes in v2:
Removed duplicated code on freeing buffers on function exit. (Thanks to Parav Pandit <[email protected]>)Fixed typo in the patch title.
Signed-off-by Long Li <[email protected]>--- fs/cifs/smb2pdu.c | 57++++++++++++++++++++++++++++++------------------------
(quoted diff) +0
-1- 1 file changed, 31 insertions(+), 26 deletions(-)fs/cifs/smb2pdu.c index renamed from fs/cifs/smb2pdu.c +0
-00f044c4..41625e4
100644fs/cifs/smb2pdu.c +0
-0@@ -729,8 +729,8 @@ SMB2_negotiate(const unsigned int xid, struct
cifs_ses
*ses)Quoted 1 line
int
smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon){
Quoted 20 lines
fs/cifs/smb2pdu.c +2
-2-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)fs/cifs/smb2pdu.c +4
-0if (tcon->ses->server->rdma)return 0 ;#endif + pneg_inbuf =kmalloc (sizeof (*pneg_inbuf),GFP_KERNEL ); +if (!pneg_inbuf) +return -ENOMEM ;/* 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)fs/cifs/smb2pdu.c
Quoted 2 lines
fs/cifs/smb2pdu.c +0
-0if (tcon->ses->session_flags &SMB2_SESSION_FLAG_IS_NULL ) cifs_dbg(VFS, "Unexpected null user (anonymous) auth flagsent by
Quoted 7 lines
server\n");
fs/cifs/smb2pdu.c +1
-1- vneg_inbuf.Capabilities = + pneg_inbuf->Capabilities = cpu_to_le32(tcon->ses->server->vals-Quoted 1 line
req_capabilities);fs/cifs/smb2pdu.c int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) · line 776
Quoted 7 lines
fs/cifs/smb2pdu.c +2
-2-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);fs/cifs/smb2pdu.c int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) · line 779
Quoted 4 lines
fs/cifs/smb2pdu.c +1
-1else if (global_secflags &CIFSSEC_MAY_SIGN ) - vneg_inbuf.SecurityMode = + pneg_inbuf->SecurityMode =cpu_to_le16(SMB2_NEGOTIATE_SIGNING_ENABLED);fs/cifs/smb2pdu.c int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) · line 785
Quoted 6 lines
fs/cifs/smb2pdu.c +1
-1else - vneg_inbuf.SecurityMode =0 ; + pneg_inbuf->SecurityMode =0 ; if (strcmp(tcon->ses->server->vals->version_string,Please check if
strncmp()should be used or not.Quoted 37 lines
fs/cifs/smb2pdu.c +10
-11SMB3ANY_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 ; } 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 ); } 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 ; } - 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),fs/cifs/smb2pdu.c
Quoted 1 line
fs/cifs/smb2pdu.c +1
-0+ (char *)pneg_inbuf, sizeof(struct
validate_negotiate_info_req),Use
sizeof(*pneg_inbuf)fs/cifs/smb2pdu.c int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) · line 814
Quoted 6 lines
fs/cifs/smb2pdu.c +1
-3(char **)&pneg_rsp, &rsplen); -if (rc !=0 ) { -cifs_dbg (VFS ,"validate protocol negotiate failed: %d\n" , rc); -return -EIO ; +if (ret !=0 ) {if (ret) is fine.
fs/cifs/smb2pdu.c int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
Quoted 6 lines
fs/cifs/smb2pdu.c +2
-1+cifs_dbg (VFS ,"validate protocol negotiate failed: %d\n" , ret); + goto out_free_inbuf; } if (rsplen != sizeof(struct validate_negotiate_info_rsp)) { @@ -820 ,7 if (rsplen !=
sizeof(*pneg_rsp))
Re: [Patch v2 2/6] cifs: Allocate validate negotiation request through kmalloc
Quoted 27 lines · Long Li via samba-technical
Quoted 27 lines
Subject: RE: [Patch v2 2/6] cifs: Allocate validate negotiation request through kmalloc
Quoted 21 lines
-----Original Message-----
From: [email protected] [mailto:linux-rdma- [email protected]] On Behalf Of Long Li
Sent: Tuesday, April 17, 2018 2:17 PM
To: Steve French <[email protected]>; [email protected]; samba- [email protected]; [email protected]; linux- [email protected]
Cc: longli <[email protected]>; [email protected]
Subject: [Patch v2 2/6] cifs: Allocate validate negotiation request through kmallocFrom: Long Li <[email protected]>
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 dialectsagainst downgrade attacks"
Quoted 7 lines
Quoted 3 lines
Format is:
Fixes ff1c038addc4 ("Check SMB3 dialects against downgrade attacks") Itshould be right above Signed-off signature.
I will fix up and resend this patch.
How about the rest patches (1, 3-6) in the series? If they don't need any changes, is it okay that I resend this one only?
Doesn't matter to me either way - I already merged patch 1 in any case.
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Changes in v2:
Removed duplicated code on freeing buffers on function exit. (Thanks to Parav Pandit <[email protected]>)
Signature · 3 lines
Thanks, Steve
RE: [Patch v2 2/6] cifs: Allocate validate negotiation request through kmalloc
In reply to Long LiQuoted 47 lines
-----Original Message-----
From: [email protected] <[email protected]> On Behalf Of Long Li
Sent: Tuesday, April 17, 2018 12:17 PM
To: Steve French <[email protected]>; [email protected]; samba- [email protected]; [email protected]; [email protected]
Cc: Long Li <[email protected]>; [email protected]
Subject: [Patch v2 2/6] cifs: Allocate validate negotiation request through kmallocFrom: Long Li <[email protected]>
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 againstdowngrade attacks"
Changes in v2:
Removed duplicated code on freeing buffers on function exit. (Thanks to Parav Pandit <[email protected]>)Fixed typo in the patch title.
Signed-off-by Long Li <[email protected]>--- fs/cifs/smb2pdu.c | 57 ++++++++++++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 26 deletions(-)fs/cifs/smb2pdu.c +2
-2@@ -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 */
fs/cifs/smb2pdu.c int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon · line 747
Quoted 7 lines
fs/cifs/smb2pdu.c +3
-0@@ -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
Quoted 111 lines
fs/cifs/smb2pdu.c +19
-19/* 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 );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 ; }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 ); }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 ; } - 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 ), (char **)&pneg_rsp, &rsplen); -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; }fs/cifs/smb2pdu.c +1
-1if (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; }fs/cifs/smb2pdu.c +6
-4/* 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
RE: [Patch v2 2/6] cifs: Allocate validate negotiation request through kmalloc
Quoted 58 lines
Subject: RE: [Patch v2 2/6] cifs: Allocate validate negotiation request through kmalloc
Quoted 55 lines
-----Original Message-----
From: [email protected] <[email protected]> On Behalf Of Long Li
Sent: Tuesday, April 17, 2018 12:17 PM
To: Steve French <[email protected]>; [email protected]; samba- [email protected]; [email protected]; [email protected]
Cc: Long Li <[email protected]>; [email protected]
Subject: [Patch v2 2/6] cifs: Allocate validate negotiation request through kmallocFrom: Long Li <[email protected]>
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 dialectsagainst downgrade attacks"
Changes in v2:
Removed duplicated code on freeing buffers on function exit. (Thanks to Parav Pandit <[email protected]>)Fixed typo in the patch title.
Signed-off-by Long Li <[email protected]>--- fs/cifs/smb2pdu.c | 57 ++++++++++++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 26 deletions(-)fs/cifs/smb2pdu.c index renamed from fs/cifs/smb2pdu.c +0
-00f044c4..41625e4 100644
fs/cifs/smb2pdu.c +2
-2@@ -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)fs/cifs/smb2pdu.c +0
-0if (tcon->ses->server->rdma)return 0 ;#endif
Quoted 8 lines
fs/cifs/smb2pdu.c int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) · line 746
Quoted 3 lines
fs/cifs/smb2pdu.c +3
-0+ 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_inbufmemory. They should instead set 'rc'
appropriately and 'goto' theout_free_inbuflabel.
Thanks!
I will move the kmalloc after those statements.
Quoted 127 lines
Michael
fs/cifs/smb2pdu.c
Quoted 3 lines
fs/cifs/smb2pdu.c +0
-0/* In SMB3.11 preauth integrity supersedes validate negotiate */ if (tcon->ses->server->dialect == SMB311_PROT_ID) @@ -764,53+767,53
fs/cifs/smb2pdu.c
Quoted 5 lines
@@ int smb3_validate_negotiate(const unsigned int xid, struct
cifs_tcon
*tcon)fs/cifs/smb2pdu.c +0
-0if (tcon->ses->session_flags &SMB2_SESSION_FLAG_IS_NULL ) cifs_dbg(VFS, "Unexpected null user (anonymous) auth flagsent by
Quoted 7 lines
server\n");
fs/cifs/smb2pdu.c +1
-1- vneg_inbuf.Capabilities = + pneg_inbuf->Capabilities = cpu_to_le32(tcon->ses->server->vals-
req_capabilities);fs/cifs/smb2pdu.c int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) · line 776
Quoted 7 lines
fs/cifs/smb2pdu.c +2
-2-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);fs/cifs/smb2pdu.c int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) · line 779
Quoted 4 lines
fs/cifs/smb2pdu.c +1
-1else if (global_secflags &CIFSSEC_MAY_SIGN ) - vneg_inbuf.SecurityMode = + pneg_inbuf->SecurityMode =cpu_to_le16(SMB2_NEGOTIATE_SIGNING_ENABLED);Quoted 32 lines
fs/cifs/smb2pdu.c +9
-9else - 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 ; }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 ); }else {/* otherwise specific dialect was requested */ - vneg_inbuf.Dialects[0 ] = + pneg_inbuf->Dialects[0 ] =cpu_to_le16 (tcon->ses->server->vals->protocol_id);fs/cifs/smb2pdu.c int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
Quoted 10 lines
fs/cifs/smb2pdu.c +2
-3- 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 ; } - 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),fs/cifs/smb2pdu.c
Quoted 1 line
fs/cifs/smb2pdu.c +1
-0+ (char *)pneg_inbuf, sizeof(struct
validate_negotiate_info_req),Quoted 12 lines
fs/cifs/smb2pdu.c +3
-4(char **)&pneg_rsp, &rsplen); -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,Quoted 3 lines
struct
cifs_tcon
*tcon)
/* relax check since Mac returns max bufsize allowed on ioctl*/
Quoted 33 lines
fs/cifs/smb2pdu.c +1
-1if ((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)fs/cifs/smb2pdu.c +2
-2/* 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 settingsmismatch\n");
fs/cifs/smb2pdu.c +4
-3-err_rsp_free: +out_free_rsp:kfree (pneg_rsp); -return -EIO ; +out_free_inbuf: +kfree (pneg_inbuf); +return rc; }enum securityEnum --2.7.4
PATCH v2 3/6 cifs: smbd: Avoid allocating iov on the stack
In reply to Long LiFrom: Long Li <[email protected]>
It's not necessary to allocate another iov when going through the buffers in smbd_send() through RDMA send.
Remove it to reduce stack size.
--- fs/cifs/smbdirect.c | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-)
fs/cifs/smbdirect.c +12 -24
@@ -2088,7 +2088,7 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst)int start, i, j;int max_iov_size = info->max_send_size -sizeof (struct smbd_data_transfer ); -struct kvec iov[SMBDIRECT_MAX_SGE ]; +struct kvec *iov;int rc; info->smbd_send_pending++;
@@ -2099,32 +2099,20 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst) }/* -* This usually means a configuration error -* We use RDMA read/write for packet size > rdma_readwrite_threshold -* as long as it's properly configured we should never get into this -* situation -*/ -if (rqst->rq_nvec + rqst->rq_npages >SMBDIRECT_MAX_SGE ) { -log_write (ERR ,"maximum send segment %x exceeding %x\n" , - rqst->rq_nvec + rqst->rq_npages,SMBDIRECT_MAX_SGE ); - rc = -EINVAL ; - goto done; - } - -/* -* Remove the RFC1002 length defined in MS-SMB2 section 2.1 -* It is used only for TCP transport +* Skip the RFC1002 length defined in MS-SMB2 section 2.1 +* It is used only for TCP transport in the iov[0] * In future we may want to add a transport layer under protocol * layer so this will only be issued to TCP transport */ - iov[0 ].iov_base = (char *)rqst->rq_iov[0 ].iov_base +4 ; - iov[0 ].iov_len = rqst->rq_iov[0 ].iov_len -4 ; - buflen += iov[0 ].iov_len; + +if (rqst->rq_iov[0 ].iov_len !=4 ) { +log_write (ERR ,"expected the pdu length in 1st iov, but got 0x%lu\n" , rqst->rq_iov[0 ].iov_len); +return -EINVAL ; + } + iov = &rqst->rq_iov[1 ];/* total up iov array first */ -for (i =1 ; i < rqst->rq_nvec; i++) { - iov[i].iov_base = rqst->rq_iov[i].iov_base; - iov[i].iov_len = rqst->rq_iov[i].iov_len; +for (i =0 ; i < rqst->rq_nvec-1 ; i++) { buflen += iov[i].iov_len; }
@@ -2197,14 +2185,14 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst) goto done; } i++; -if (i == rqst->rq_nvec) +if (i == rqst->rq_nvec-1 )break ; } start = i; buflen =0 ; }else { i++; -if (i == rqst->rq_nvec) { +if (i == rqst->rq_nvec-1 ) {/* send out all remaining vecs */ remaining_data_length -= buflen; log_write(INFO,
Signature · 1 line
2.7.4
Re: [Patch v2 3/6] cifs: smbd: Avoid allocating iov on the stack
Didn't see any obvious problems, but can you fix the checkpatch warnings and resend to the list (I am more concerned about the last two warnings rather than the first one).
$ scripts/checkpatch.pl 0001-cifs-smbd-Avoid-allocating-iov-on-the-stack.patch WARNING: line over 80 characters #60: FILE: fs/cifs/smbdirect.c:2106: + log_write(ERR, "expected the pdu length in 1st iov, but got 0x%lu\n", rqst->rq_iov[0].iov_len);
ERROR: Prefixing 0x with decimal output is defective #60: FILE: fs/cifs/smbdirect.c:2106:
+ log_write(ERR, "expected the pdu length in 1st iov, but got
0x%lu\n", rqst->rq_iov[0].iov_len);
WARNING: braces {} are not necessary for single statement blocks #69: FILE: fs/cifs/smbdirect.c:2112:
+ for (i = 0; i < rqst->rq_nvec-1; i++) {
buflen += iov[i].iov_len;
}total: 1 errors, 2 warnings, 65 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
0001-cifs-smbd-Avoid-allocating-iov-on-the-stack.patch has style problems, please review.
Quoted 92 lines · Long Li
From: Long Li <[email protected]>
It's not necessary to allocate another iov when going through the buffers in
smbd_send()through RDMA send.Remove it to reduce stack size.
Signed-off-by Long Li <[email protected]>--- fs/cifs/smbdirect.c | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-)fs/cifs/smbdirect.c +12
-25@@ -2088,7 +2088,7 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst)int start, i, j;int max_iov_size = info->max_send_size -sizeof (struct smbd_data_transfer ); -struct kvec iov[SMBDIRECT_MAX_SGE ]; +struct kvec *iov;int rc; info->smbd_send_pending++;@@ -2099,32 +2099,20 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst) }/* -* This usually means a configuration error -* We use RDMA read/write for packet size > rdma_readwrite_threshold -* as long as it's properly configured we should never get into this -* situation -*/ -if (rqst->rq_nvec + rqst->rq_npages >SMBDIRECT_MAX_SGE ) { -log_write (ERR ,"maximum send segment %x exceeding %x\n" , - rqst->rq_nvec + rqst->rq_npages,SMBDIRECT_MAX_SGE ); - rc = -EINVAL ; - goto done; - } - -/* -* Remove the RFC1002 length defined in MS-SMB2 section 2.1 -* It is used only for TCP transport +* Skip the RFC1002 length defined in MS-SMB2 section 2.1 +* It is used only for TCP transport in the iov[0] * In future we may want to add a transport layer under protocol * layer so this will only be issued to TCP transport */ - iov[0 ].iov_base = (char *)rqst->rq_iov[0 ].iov_base +4 ; - iov[0 ].iov_len = rqst->rq_iov[0 ].iov_len -4 ; - buflen += iov[0 ].iov_len; + +if (rqst->rq_iov[0 ].iov_len !=4 ) { +log_write (ERR ,"expected the pdu length in 1st iov, but got 0x%lu\n" , rqst->rq_iov[0 ].iov_len); +return -EINVAL ; + } + iov = &rqst->rq_iov[1 ];/* total up iov array first */ -for (i =1 ; i < rqst->rq_nvec; i++) { - iov[i].iov_base = rqst->rq_iov[i].iov_base; - iov[i].iov_len = rqst->rq_iov[i].iov_len; +for (i =0 ; i < rqst->rq_nvec-1 ; i++) { buflen += iov[i].iov_len; }@@ -2197,14 +2185,14 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst) goto done; } i++; -if (i == rqst->rq_nvec) +if (i == rqst->rq_nvec-1 )break ; } start = i; buflen =0 ; }else { i++; -if (i == rqst->rq_nvec) { +if (i == rqst->rq_nvec-1 ) {/* send out all remaining vecs */ remaining_data_length -= buflen; log_write(INFO, --2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 3 lines
Thanks, Steve
RE: [Patch v2 3/6] cifs: smbd: Avoid allocating iov on the stack
Quoted 5 lines
Subject: Re: [Patch v2 3/6] cifs: smbd: Avoid allocating iov on the stack
Didn't see any obvious problems, but can you fix the checkpatch warnings and resend to the list (I am more concerned about the last two warnings rather than the first one).
Yes, I will fix it and resend.
Quoted 153 lines
$ scripts/checkpatch.pl 0001-cifs-smbd-Avoid-allocating-iov-on-the- stack.patch WARNING: line over 80 characters #60: FILE: fs/cifs/smbdirect.c:2106: + log_write(ERR, "expected the pdu length in 1st iov, but got 0x%lu\n", rqst->rq_iov[0].iov_len);ERROR: Prefixing 0x with decimal output is defective #60: FILE:
fs/cifs/smbdirect.c:2106:
+ log_write(ERR, "expected the pdu length in 1st iov, but got
0x%lu\n", rqst->rq_iov[0].iov_len);WARNING: braces {} are not necessary for single statement blocks #69: FILE:
fs/cifs/smbdirect.c:2112:+ for (i = 0; i < rqst->rq_nvec-1; i++) { buflen += iov[i].iov_len; }total: 1 errors, 2 warnings, 65 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.0001-cifs-smbd-Avoid-allocating-iov-on-the-stack.patch has style problems, please review.
fs/cifs/smbdirect.c int smbd_send(struct smbd_connection *info,
Quoted 18 lines · Long Li
From: Long Li <[email protected]>
It's not necessary to allocate another iov when going through the buffers in
smbd_send()through RDMA send.Remove it to reduce stack size.
Signed-off-by Long Li <[email protected]>--- fs/cifs/smbdirect.c | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-)fs/cifs/smbdirect.c index renamed from fs/cifs/smbdirect.c +0
-0b5c6c0d..f575e9a 100644
fs/cifs/smbdirect.c +0
-0@@ -2088,7 +2088,7 @@ int smbd_send(struct smbd_connection *info,struct
smb_rqst*rqst)fs/cifs/smbdirect.c int smbd_send(struct smbd_connection *info,
Quoted 9 lines
fs/cifs/smbdirect.c +1
-1int start, i, j;int max_iov_size = info->max_send_size -sizeof (struct smbd_data_transfer ); -struct kvec iov[SMBDIRECT_MAX_SGE ]; +struct kvec *iov;int rc; info->smbd_send_pending++;@@ -2099,32 +2099,20 @@ int smbd_send(struct smbd_connection *info,struct
smb_rqst*rqst)fs/cifs/smbdirect.c int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst)
Quoted 5 lines
fs/cifs/smbdirect.c +0
-2}/* -* This usually means a configuration error -* We use RDMA read/write for packet size >
rdma_readwrite_thresholdQuoted 14 lines
fs/cifs/smbdirect.c +1
-13-* as long as it's properly configured we should never get into this -* situation -*/ -if (rqst->rq_nvec + rqst->rq_npages >SMBDIRECT_MAX_SGE ) { -log_write (ERR ,"maximum send segment %x exceeding %x\n" , - rqst->rq_nvec + rqst->rq_npages,SMBDIRECT_MAX_SGE ); - rc = -EINVAL ; - goto done; - } - -/* -* Remove the RFC1002 length defined in MS-SMB2 section 2.1 -* It is used only for TCP transport +* Skip the RFC1002 length defined in MS-SMB2 section 2.1 fs/cifs/smbdirect.c int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst)
Quoted 10 lines
fs/cifs/smbdirect.c +4
-3+* It is used only for TCP transport in the iov[0] * In future we may want to add a transport layer under protocol * layer so this will only be issued to TCP transport */ - iov[0 ].iov_base = (char *)rqst->rq_iov[0 ].iov_base +4 ; - iov[0 ].iov_len = rqst->rq_iov[0 ].iov_len -4 ; - buflen += iov[0 ].iov_len; + +if (rqst->rq_iov[0 ].iov_len !=4 ) { + log_write(ERR, "expected the pdu length in 1st iov, but got0x%lu\n", rqst->rq_iov[0].iov_len);
fs/cifs/smbdirect.c int smbd_send(struct smbd_connection *info,
Quoted 13 lines
fs/cifs/smbdirect.c +4
-3+return -EINVAL ; + } + iov = &rqst->rq_iov[1 ];/* total up iov array first */ -for (i =1 ; i < rqst->rq_nvec; i++) { - iov[i].iov_base = rqst->rq_iov[i].iov_base; - iov[i].iov_len = rqst->rq_iov[i].iov_len; +for (i =0 ; i < rqst->rq_nvec-1 ; i++) { buflen += iov[i].iov_len; }@@ -2197,14 +2185,14 @@ int smbd_send(struct smbd_connection *info,struct
smb_rqst*rqst)Quoted 19 lines
fs/cifs/smbdirect.c +2
-3goto done; } i++; -if (i == rqst->rq_nvec) +if (i == rqst->rq_nvec-1 )break ; } start = i; buflen =0 ; }else { i++; -if (i == rqst->rq_nvec) { +if (i == rqst->rq_nvec-1 ) {/* send out all remaining vecs */ remaining_data_length -= buflen; log_write(INFO, --2.7.4
Quoted 3 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] Moremajordomo
Quoted 2 lines
info at
https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.k
Quoted 1 line
ernel.org%2Fmajordomo-
info.html&data=02%7C01%7Clongli%40microsoft.com%
Quoted 1 line
7C23fbdbb0598a497f624708d5a92f6435%7C72f988bf86f141af91ab2d7cd011d b47%
Quoted 1 line
7C1%7C0%7C636600943380887988&sdata=imh9cWDQQEXqxHdUrCuEykgiLfKs UAl20jB
Quoted 1 line
yPOS7FrI%3D&reserved=0
--
Thanks,Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at
https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.ke
rnel.org%2Fmajordomo- info.html&data=02%7C01%7Clongli%40microsoft.com%7C23fbdbb0598a497f 624708d5a92f6435%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63 6600943380887988&sdata=imh9cWDQQEXqxHdUrCuEykgiLfKsUAl20jByPOS7F rI%3D&reserved=0
Re: [Patch v2 3/6] cifs: smbd: Avoid allocating iov on the stack
I am ok with the other style warning - so I just fixed up the one error (see attached) and merged into cifs-2.6.git for-next
That leaves two more from your series to review (and for others to review). Let's just focus on those two.
Quoted 161 lines · Long Li
Quoted 5 lines
Subject: Re: [Patch v2 3/6] cifs: smbd: Avoid allocating iov on the stack
Didn't see any obvious problems, but can you fix the checkpatch warnings and resend to the list (I am more concerned about the last two warnings rather than the first one).
Yes, I will fix it and resend.
Quoted 153 lines
$ scripts/checkpatch.pl 0001-cifs-smbd-Avoid-allocating-iov-on-the- stack.patch WARNING: line over 80 characters #60: FILE: fs/cifs/smbdirect.c:2106: + log_write(ERR, "expected the pdu length in 1st iov, but got 0x%lu\n", rqst->rq_iov[0].iov_len);ERROR: Prefixing 0x with decimal output is defective #60: FILE:
fs/cifs/smbdirect.c:2106:
+ log_write(ERR, "expected the pdu length in 1st iov, but got
0x%lu\n", rqst->rq_iov[0].iov_len);WARNING: braces {} are not necessary for single statement blocks #69: FILE:
fs/cifs/smbdirect.c:2112:+ for (i = 0; i < rqst->rq_nvec-1; i++) { buflen += iov[i].iov_len; }total: 1 errors, 2 warnings, 65 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.0001-cifs-smbd-Avoid-allocating-iov-on-the-stack.patch has style problems, please review.
fs/cifs/smbdirect.c int smbd_send(struct smbd_connection *info,
Quoted 18 lines · Long Li
From: Long Li <[email protected]>
It's not necessary to allocate another iov when going through the buffers in
smbd_send()through RDMA send.Remove it to reduce stack size.
Signed-off-by Long Li <[email protected]>--- fs/cifs/smbdirect.c | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-)fs/cifs/smbdirect.c index renamed from fs/cifs/smbdirect.c +0
-0b5c6c0d..f575e9a 100644
fs/cifs/smbdirect.c +0
-0@@ -2088,7 +2088,7 @@ int smbd_send(struct smbd_connection *info,struct
smb_rqst*rqst)fs/cifs/smbdirect.c int smbd_send(struct smbd_connection *info,
Quoted 9 lines
fs/cifs/smbdirect.c +1
-1int start, i, j;int max_iov_size = info->max_send_size -sizeof (struct smbd_data_transfer ); -struct kvec iov[SMBDIRECT_MAX_SGE ]; +struct kvec *iov;int rc; info->smbd_send_pending++;@@ -2099,32 +2099,20 @@ int smbd_send(struct smbd_connection *info,struct
smb_rqst*rqst)fs/cifs/smbdirect.c int smbd_send(struct smbd_connection *info, · line 2103
Quoted 5 lines
fs/cifs/smbdirect.c +0
-2}/* -* This usually means a configuration error -* We use RDMA read/write for packet size >
rdma_readwrite_thresholdQuoted 14 lines
fs/cifs/smbdirect.c +1
-13-* as long as it's properly configured we should never get into this -* situation -*/ -if (rqst->rq_nvec + rqst->rq_npages >SMBDIRECT_MAX_SGE ) { -log_write (ERR ,"maximum send segment %x exceeding %x\n" , - rqst->rq_nvec + rqst->rq_npages,SMBDIRECT_MAX_SGE ); - rc = -EINVAL ; - goto done; - } - -/* -* Remove the RFC1002 length defined in MS-SMB2 section 2.1 -* It is used only for TCP transport +* Skip the RFC1002 length defined in MS-SMB2 section 2.1 fs/cifs/smbdirect.c int smbd_send(struct smbd_connection *info, · line 2109
Quoted 10 lines
fs/cifs/smbdirect.c +4
-3+* It is used only for TCP transport in the iov[0] * In future we may want to add a transport layer under protocol * layer so this will only be issued to TCP transport */ - iov[0 ].iov_base = (char *)rqst->rq_iov[0 ].iov_base +4 ; - iov[0 ].iov_len = rqst->rq_iov[0 ].iov_len -4 ; - buflen += iov[0 ].iov_len; + +if (rqst->rq_iov[0 ].iov_len !=4 ) { + log_write(ERR, "expected the pdu length in 1st iov, but got0x%lu\n", rqst->rq_iov[0].iov_len);
fs/cifs/smbdirect.c int smbd_send(struct smbd_connection *info,
Quoted 13 lines
fs/cifs/smbdirect.c +4
-3+return -EINVAL ; + } + iov = &rqst->rq_iov[1 ];/* total up iov array first */ -for (i =1 ; i < rqst->rq_nvec; i++) { - iov[i].iov_base = rqst->rq_iov[i].iov_base; - iov[i].iov_len = rqst->rq_iov[i].iov_len; +for (i =0 ; i < rqst->rq_nvec-1 ; i++) { buflen += iov[i].iov_len; }@@ -2197,14 +2185,14 @@ int smbd_send(struct smbd_connection *info,struct
smb_rqst*rqst)Quoted 19 lines
fs/cifs/smbdirect.c +2
-3goto done; } i++; -if (i == rqst->rq_nvec) +if (i == rqst->rq_nvec-1 )break ; } start = i; buflen =0 ; }else { i++; -if (i == rqst->rq_nvec) { +if (i == rqst->rq_nvec-1 ) {/* send out all remaining vecs */ remaining_data_length -= buflen; log_write(INFO, --2.7.4
Quoted 3 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] Moremajordomo
Quoted 2 lines
info at
https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.k
Quoted 1 line
ernel.org%2Fmajordomo-
info.html&data=02%7C01%7Clongli%40microsoft.com%
Quoted 1 line
7C23fbdbb0598a497f624708d5a92f6435%7C72f988bf86f141af91ab2d7cd011d b47%
Quoted 1 line
7C1%7C0%7C636600943380887988&sdata=imh9cWDQQEXqxHdUrCuEykgiLfKs UAl20jB
Quoted 1 line
yPOS7FrI%3D&reserved=0
--
Thanks,Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at
https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.ke
rnel.org%2Fmajordomo- info.html&data=02%7C01%7Clongli%40microsoft.com%7C23fbdbb0598a497f 624708d5a92f6435%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63 6600943380887988&sdata=imh9cWDQQEXqxHdUrCuEykgiLfKsUAl20jByPOS7F rI%3D&reserved=0
Signature · 3 lines
Thanks, Steve
From 553018657de265a7c999f4fd12864fd0646319a1 Mon Sep 17 00:00:00 2001
From: Long Li <[email protected]>
Date: Tue, 17 Apr 2018 12:17:07 -0700
Subject: [PATCH] cifs: smbd: Avoid allocating iov on the stack
It's not necessary to allocate another iov when going through the buffers in smbd_send() through RDMA send.
Remove it to reduce stack size.
--- fs/cifs/smbdirect.c | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-)
fs/cifs/smbdirect.c +12 -24
@@ -2086,7 +2086,7 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst)int start, i, j;int max_iov_size = info->max_send_size -sizeof (struct smbd_data_transfer ); -struct kvec iov[SMBDIRECT_MAX_SGE ]; +struct kvec *iov;int rc; info->smbd_send_pending++;
@@ -2096,32 +2096,20 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst) }/* -* This usually means a configuration error -* We use RDMA read/write for packet size > rdma_readwrite_threshold -* as long as it's properly configured we should never get into this -* situation -*/ -if (rqst->rq_nvec + rqst->rq_npages >SMBDIRECT_MAX_SGE ) { -log_write (ERR ,"maximum send segment %x exceeding %x\n" , - rqst->rq_nvec + rqst->rq_npages,SMBDIRECT_MAX_SGE ); - rc = -EINVAL ; - goto done; - } - -/* -* Remove the RFC1002 length defined in MS-SMB2 section 2.1 -* It is used only for TCP transport +* Skip the RFC1002 length defined in MS-SMB2 section 2.1 +* It is used only for TCP transport in the iov[0] * In future we may want to add a transport layer under protocol * layer so this will only be issued to TCP transport */ - iov[0 ].iov_base = (char *)rqst->rq_iov[0 ].iov_base +4 ; - iov[0 ].iov_len = rqst->rq_iov[0 ].iov_len -4 ; - buflen += iov[0 ].iov_len; + +if (rqst->rq_iov[0 ].iov_len !=4 ) { +log_write (ERR ,"expected the pdu length in 1st iov, but got %lu\n" , rqst->rq_iov[0 ].iov_len); +return -EINVAL ; + } + iov = &rqst->rq_iov[1 ];/* total up iov array first */ -for (i =1 ; i < rqst->rq_nvec; i++) { - iov[i].iov_base = rqst->rq_iov[i].iov_base; - iov[i].iov_len = rqst->rq_iov[i].iov_len; +for (i =0 ; i < rqst->rq_nvec-1 ; i++) { buflen += iov[i].iov_len; }
@@ -2198,14 +2186,14 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst) goto done; } i++; -if (i == rqst->rq_nvec) +if (i == rqst->rq_nvec-1 )break ; } start = i; buflen =0 ; }else { i++; -if (i == rqst->rq_nvec) { +if (i == rqst->rq_nvec-1 ) {/* send out all remaining vecs */ remaining_data_length -= buflen; log_write(INFO,
Signature · 1 line
2.14.1
PATCH v2 4/6 cifs: smbd: Don't use RDMA read/write when signing is used
In reply to Long LiFrom: Long Li <[email protected]>
SMB server will not sign data transferred through RDMA read/write. When signing is used, it's a good idea to have all the data signed.
In this case, use RDMA send/recv for all data transfers. This will degrade performance as this is not generally configured in RDMA environemnt. So warn the user on signing and RDMA send/recv.
--- fs/cifs/cifssmb.c | 3 +++ fs/cifs/smb2ops.c | 18 ++++++++++++++---- fs/cifs/smb2pdu.c | 4 ++-- 3 files changed, 19 insertions(+), 6 deletions(-)
fs/cifs/cifssmb.c +3 -0
@@ -455,6 +455,9 @@ cifs_enable_signing(struct TCP_Server_Info *server, bool mnt_sign_required) server->sign = true; } +if (cifs_rdma_enabled (server) && server->sign) +cifs_dbg (VFS ,"Signing is enabled, and RDMA read/write will be disabled" ); +return 0 ; }
fs/cifs/smb2ops.c +14 -4
@@ -252,9 +252,14 @@ smb2_negotiate_wsize(struct cifs_tcon *tcon, struct smb_vol *volume_info) wsize = volume_info->wsize ? volume_info->wsize :CIFS_DEFAULT_IOSIZE ; wsize = min_t(unsigned int, wsize, server->max_write);#ifdef CONFIG_CIFS_SMB_DIRECT -if (server->rdma) - wsize = min_t(unsigned int, +if (server->rdma) { +if (server->sign) + wsize = min_t(unsigned int, + wsize, server->smbd_conn->max_fragmented_send_size); +else + wsize = min_t(unsigned int, wsize, server->smbd_conn->max_readwrite_size); + }#endif if (!(server->capabilities &SMB2_GLOBAL_CAP_LARGE_MTU )) wsize = min_t(unsigned int, wsize, SMB2_MAX_BUFFER_SIZE);
@@ -272,9 +277,14 @@ smb2_negotiate_rsize(struct cifs_tcon *tcon, struct smb_vol *volume_info) rsize = volume_info->rsize ? volume_info->rsize :CIFS_DEFAULT_IOSIZE ; rsize = min_t(unsigned int, rsize, server->max_read);#ifdef CONFIG_CIFS_SMB_DIRECT -if (server->rdma) - rsize = min_t(unsigned int, +if (server->rdma) { +if (server->sign) + rsize = min_t(unsigned int, + rsize, server->smbd_conn->max_fragmented_recv_size); +else + rsize = min_t(unsigned int, rsize, server->smbd_conn->max_readwrite_size); + }#endif if (!(server->capabilities &SMB2_GLOBAL_CAP_LARGE_MTU ))
fs/cifs/smb2pdu.c +2 -2
@@ -2595,7 +2595,7 @@ smb2_new_read_req(void **buf, unsigned int *total_len,* If we want to do a RDMA write, fill in and append * smbd_buffer_descriptor_v1 to the end of read request */ -if (server->rdma && rdata && +if (server->rdma && rdata && !server->sign && rdata->bytes >= server->smbd_conn->rdma_readwrite_threshold) {struct smbd_buffer_descriptor_v1 *v1;
@@ -2973,7 +2973,7 @@ smb2_async_writev(struct cifs_writedata *wdata,* If we want to do a server RDMA read, fill in and append * smbd_buffer_descriptor_v1 to the end of write request */ -if (server->rdma && wdata->bytes >= +if (server->rdma && !server->sign && wdata->bytes >= server->smbd_conn->rdma_readwrite_threshold) {struct smbd_buffer_descriptor_v1 *v1;
Signature · 1 line
2.7.4
Steve French
AppliedRe: [Patch v2 4/6] cifs: smbd: Don't use RDMA read/write when signing is used
merged into cifs-2.6.git for-next
Quoted 98 lines · Long Li
From: Long Li <[email protected]>
SMB server will not sign data transferred through RDMA read/write. When signing is used, it's a good idea to have all the data signed.
In this case, use RDMA send/recv for all data transfers. This will degrade performance as this is not generally configured in RDMA environemnt. So warn the user on signing and RDMA send/recv.
Signed-off-by Long Li <[email protected]>--- fs/cifs/cifssmb.c | 3 +++ fs/cifs/smb2ops.c | 18 ++++++++++++++---- fs/cifs/smb2pdu.c | 4 ++-- 3 files changed, 19 insertions(+), 6 deletions(-)fs/cifs/cifssmb.c +3
-0@@ -455,6 +455,9 @@ cifs_enable_signing(struct TCP_Server_Info *server, bool mnt_sign_required) server->sign = true; } +if (cifs_rdma_enabled (server) && server->sign) +cifs_dbg (VFS ,"Signing is enabled, and RDMA read/write will be disabled" ); +return 0 ; }fs/cifs/smb2ops.c +14
-4@@ -252,9 +252,14 @@ smb2_negotiate_wsize(struct cifs_tcon *tcon, struct smb_vol *volume_info) wsize = volume_info->wsize ? volume_info->wsize :CIFS_DEFAULT_IOSIZE ; wsize = min_t(unsigned int, wsize, server->max_write);#ifdef CONFIG_CIFS_SMB_DIRECT -if (server->rdma) - wsize = min_t(unsigned int, +if (server->rdma) { +if (server->sign) + wsize = min_t(unsigned int, + wsize, server->smbd_conn->max_fragmented_send_size); +else + wsize = min_t(unsigned int, wsize, server->smbd_conn->max_readwrite_size); + }#endif if (!(server->capabilities &SMB2_GLOBAL_CAP_LARGE_MTU )) wsize = min_t(unsigned int, wsize, SMB2_MAX_BUFFER_SIZE);@@ -272,9 +277,14 @@ smb2_negotiate_rsize(struct cifs_tcon *tcon, struct smb_vol *volume_info) rsize = volume_info->rsize ? volume_info->rsize :CIFS_DEFAULT_IOSIZE ; rsize = min_t(unsigned int, rsize, server->max_read);#ifdef CONFIG_CIFS_SMB_DIRECT -if (server->rdma) - rsize = min_t(unsigned int, +if (server->rdma) { +if (server->sign) + rsize = min_t(unsigned int, + rsize, server->smbd_conn->max_fragmented_recv_size); +else + rsize = min_t(unsigned int, rsize, server->smbd_conn->max_readwrite_size); + }#endif if (!(server->capabilities &SMB2_GLOBAL_CAP_LARGE_MTU ))fs/cifs/smb2pdu.c +2
-3@@ -2595,7 +2595,7 @@ smb2_new_read_req(void **buf, unsigned int *total_len,* If we want to do a RDMA write, fill in and append * smbd_buffer_descriptor_v1 to the end of read request */ -if (server->rdma && rdata && +if (server->rdma && rdata && !server->sign && rdata->bytes >= server->smbd_conn->rdma_readwrite_threshold) {struct smbd_buffer_descriptor_v1 *v1;@@ -2973,7 +2973,7 @@ smb2_async_writev(struct cifs_writedata *wdata,* If we want to do a server RDMA read, fill in and append * smbd_buffer_descriptor_v1 to the end of write request */ -if (server->rdma && wdata->bytes >= +if (server->rdma && !server->sign && wdata->bytes >= server->smbd_conn->rdma_readwrite_threshold) {struct smbd_buffer_descriptor_v1 *v1; --2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 3 lines
Thanks, Steve
PATCH v2 5/6 cifs: smbd: Enable signing with smbdirect
In reply to Long LiFrom: 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 | 4 ---- 2 files changed, 12 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 -4
@@ -737,10 +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 pneg_inbuf =kmalloc (sizeof (*pneg_inbuf),GFP_KERNEL );if (!pneg_inbuf)return -ENOMEM ;
Signature · 1 line
2.7.4
PATCH v2 6/6 cifs: smbd: Dump SMB packet when configured
In reply to Long LiFrom: Long Li <[email protected]>
When sending through SMB Direct, also dump the packet in SMB send path.
Also fixed a typo in debug message.
--- fs/cifs/smbdirect.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
fs/cifs/smbdirect.c +5 -1
@@ -1029,7 +1029,7 @@ static int smbd_post_send(struct smbd_connection *info,for (i =0 ; i < request->num_sge; i++) {log_rdma_send (INFO ,"rdma_request sge[%d] addr=%llu length=%u\n" , - i, request->sge[0 ].addr, request->sge[0 ].length); + i, request->sge[i].addr, request->sge[i].length); ib_dma_sync_single_for_device( info->id->device, request->sge[i].addr,
@@ -2130,6 +2130,10 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst) goto done; } +cifs_dbg (FYI ,"Sending smb (RDMA): smb_len=%u\n" , buflen); +for (i =0 ; i < rqst->rq_nvec-1 ; i++) +dump_smb (iov[i].iov_base, iov[i].iov_len); + remaining_data_length = buflen; log_write(INFO, "rqst->rq_nvec=%d rqst->rq_npages=%d rq_pagesz=%d "
Signature · 1 line
2.7.4
Quoted 11 lines
-----Original Message-----
From: [email protected] <[email protected]> On Behalf Of Long Li
Sent: Tuesday, April 17, 2018 12:17 PM
To: Steve French <[email protected]>; [email protected]; samba- [email protected]; [email protected]; [email protected]
Cc: Long Li <[email protected]>; [email protected]
Subject: [Patch v2 1/6] cifs: smbd: Check for iov length on sending the last iovFrom: Long Li <[email protected]>
Quoted 7 lines
When sending the last iov that breaks into smaller buffers to fit the transfer size, it's necessary to check if this is the last iov.
If this is the latest iov, stop and proceed to send pages.
Signed-off-by Long Li <[email protected]>
But a question unrelated to this change arose during my review: At the beginning and end of smbd_send(), the field smbd_send_pending is
incremented and decremented, respectively. The increment/decrement
are not done as atomic operations. Is this code guaranteed to be single threaded? If not, the count could become corrupted, and smbd_destroy_rdma_work(), which waits for the count to become zero, could hang. A similar question applies to smbd_recv_pending in smbd_recv().
Quoted 19 lines
--- fs/cifs/smbdirect.c | 2 ++ 1 file changed, 2 insertions(+)fs/cifs/smbdirect.c +2
-1@@ -2197,6 +2197,8 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst) goto done; } i++; +if (i == rqst->rq_nvec) +break ; } start = i; buflen =0 ; --2.7.4
Quoted 26 lines
Subject: RE: [Patch v2 1/6] cifs: smbd: Check for iov length on sending the last iov
Quoted 13 lines
-----Original Message-----
From: [email protected] <[email protected]> On Behalf Of Long Li
Sent: Tuesday, April 17, 2018 12:17 PM
To: Steve French <[email protected]>; [email protected]; samba- [email protected]; [email protected]; [email protected]
Cc: Long Li <[email protected]>; [email protected]
Subject: [Patch v2 1/6] cifs: smbd: Check for iov length on sending the last iovFrom: Long Li <[email protected]>
Quoted 7 lines
When sending the last iov that breaks into smaller buffers to fit the transfer size, it's necessary to check if this is the last iov.
If this is the latest iov, stop and proceed to send pages.
Signed-off-by Long Li <[email protected]>Reviewed-by Michael Kelley <[email protected]>
Quoted 7 lines
But a question unrelated to this change arose during my review: At the beginning and end of
smbd_send(), the fieldsmbd_send_pendingis
incremented and decremented, respectively. The increment/decrement
are not done as atomic operations. Is this code guaranteed to be single threaded? If not, the count could become corrupted, andsmbd_destroy_rdma_work(), which waits for the count to become zero, could hang. A similar question applies tosmbd_recv_pendinginsmbd_recv().
It is safe. The transport sending path is locked by TCP_Server_Info->srv_mutex.
The transport receiving path is single threaded.
Quoted 21 lines
fs/cifs/smbdirect.c int smbd_send(struct smbd_connection *info,
Quoted 10 lines
--- fs/cifs/smbdirect.c | 2 ++ 1 file changed, 2 insertions(+)fs/cifs/smbdirect.c index renamed from fs/cifs/smbdirect.c +0
-090e673c..b5c6c0d 100644
fs/cifs/smbdirect.c +0
-0@@ -2197,6 +2197,8 @@ int smbd_send(struct smbd_connection *info,struct
smb_rqst*rqst)Quoted 10 lines
fs/cifs/smbdirect.c +2
-1goto done; } i++; +if (i == rqst->rq_nvec) +break ; } start = i; buflen =0 ; --2.7.4