| Message ID | 20180615154128.20981-1-paulo@paulo.ac |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] cifs: Use correct packet length in SMB2_TRANSFORM header | expand |
Good catch! Refactoring looks good to me.
Reviewed-by: Aurelien Aptel <aaptel@suse.com>
Got a build break with it
commit 6c680d47cf25aecdb31b608b4d4191b086b71ccf
Author: Paulo Alcantara <paulo@paulo.ac>
Date: Fri Jun 15 12:41:28 2018 -0300
cifs: Use correct packet length in SMB2_TRANSFORM header
CC [M] fs/cifs/smbdirect.o
fs/cifs/smbdirect.c: In function ‘smbd_send’:
fs/cifs/smbdirect.c:2120:11: error: implicit declaration of function
‘smb2_rqst_len’; did you mean ‘smbd_post_send’?
[-Werror=implicit-function-declaration]
buflen = smb2_rqst_len(rqst, true);
^~~~~~~~~~~~~
smbd_post_send
On Fri, Jun 15, 2018 at 10:55 AM, Aurélien Aptel <aaptel@suse.com> wrote:
>
> Good catch! Refactoring looks good to me.
>
> Reviewed-by: Aurelien Aptel <aaptel@suse.com>
>
> --
> Aurélien Aptel / SUSE Labs Samba Team
> GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3
> SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
Steve French <smfrench@gmail.com> writes: > Got a build break with it > > commit 6c680d47cf25aecdb31b608b4d4191b086b71ccf > Author: Paulo Alcantara <paulo@paulo.ac> > Date: Fri Jun 15 12:41:28 2018 -0300 > > cifs: Use correct packet length in SMB2_TRANSFORM header > > CC [M] fs/cifs/smbdirect.o > fs/cifs/smbdirect.c: In function ‘smbd_send’: > fs/cifs/smbdirect.c:2120:11: error: implicit declaration of function > ‘smb2_rqst_len’; did you mean ‘smbd_post_send’? > [-Werror=implicit-function-declaration] > buflen = smb2_rqst_len(rqst, true); > ^~~~~~~~~~~~~ > smbd_post_send Whoops - it seems that smbdirect.c missed a: #include "smb2proto.h" Could you please do it and see if it works now? Thanks Paulo -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 15, 2018 at 12:51 PM, Paulo Alcantara <paulo@paulo.ac> wrote: > Steve French <smfrench@gmail.com> writes: > >> Got a build break with it >> >> commit 6c680d47cf25aecdb31b608b4d4191b086b71ccf >> Author: Paulo Alcantara <paulo@paulo.ac> >> Date: Fri Jun 15 12:41:28 2018 -0300 >> >> cifs: Use correct packet length in SMB2_TRANSFORM header >> >> CC [M] fs/cifs/smbdirect.o >> fs/cifs/smbdirect.c: In function ‘smbd_send’: >> fs/cifs/smbdirect.c:2120:11: error: implicit declaration of function >> ‘smb2_rqst_len’; did you mean ‘smbd_post_send’? >> [-Werror=implicit-function-declaration] >> buflen = smb2_rqst_len(rqst, true); >> ^~~~~~~~~~~~~ >> smbd_post_send > > Whoops - it seems that smbdirect.c missed a: #include "smb2proto.h" > > Could you please do it and see if it works now? Yes - as expected that fixed it. Can you resend me (offline is fine) - with the reviewed bys added (Aurelien et al)
Steve French <smfrench@gmail.com> writes: > On Fri, Jun 15, 2018 at 12:51 PM, Paulo Alcantara <paulo@paulo.ac> wrote: >> Steve French <smfrench@gmail.com> writes: >> >>> Got a build break with it >>> >>> commit 6c680d47cf25aecdb31b608b4d4191b086b71ccf >>> Author: Paulo Alcantara <paulo@paulo.ac> >>> Date: Fri Jun 15 12:41:28 2018 -0300 >>> >>> cifs: Use correct packet length in SMB2_TRANSFORM header >>> >>> CC [M] fs/cifs/smbdirect.o >>> fs/cifs/smbdirect.c: In function ‘smbd_send’: >>> fs/cifs/smbdirect.c:2120:11: error: implicit declaration of function >>> ‘smb2_rqst_len’; did you mean ‘smbd_post_send’? >>> [-Werror=implicit-function-declaration] >>> buflen = smb2_rqst_len(rqst, true); >>> ^~~~~~~~~~~~~ >>> smbd_post_send >> >> Whoops - it seems that smbdirect.c missed a: #include "smb2proto.h" >> >> Could you please do it and see if it works now? > > Yes - as expected that fixed it. Can you resend me (offline is fine) > - with the reviewed bys added (Aurelien et al) Sure. Thanks! Paulo -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
updated patch merged to cifs-2.6.git along with the other fix for Paulo - good job - thx On Fri, Jun 15, 2018 at 2:57 PM, Paulo Alcantara <paulo@paulo.ac> wrote: > Steve French <smfrench@gmail.com> writes: > >> On Fri, Jun 15, 2018 at 12:51 PM, Paulo Alcantara <paulo@paulo.ac> wrote: >>> Steve French <smfrench@gmail.com> writes: >>> >>>> Got a build break with it >>>> >>>> commit 6c680d47cf25aecdb31b608b4d4191b086b71ccf >>>> Author: Paulo Alcantara <paulo@paulo.ac> >>>> Date: Fri Jun 15 12:41:28 2018 -0300 >>>> >>>> cifs: Use correct packet length in SMB2_TRANSFORM header >>>> >>>> CC [M] fs/cifs/smbdirect.o >>>> fs/cifs/smbdirect.c: In function ‘smbd_send’: >>>> fs/cifs/smbdirect.c:2120:11: error: implicit declaration of function >>>> ‘smb2_rqst_len’; did you mean ‘smbd_post_send’? >>>> [-Werror=implicit-function-declaration] >>>> buflen = smb2_rqst_len(rqst, true); >>>> ^~~~~~~~~~~~~ >>>> smbd_post_send >>> >>> Whoops - it seems that smbdirect.c missed a: #include "smb2proto.h" >>> >>> Could you please do it and see if it works now? >> >> Yes - as expected that fixed it. Can you resend me (offline is fine) >> - with the reviewed bys added (Aurelien et al) > > Sure. Thanks! > > Paulo
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index badcfb2f3c22..0356b5559c71 100644 --- a/fs/cifs/smb2ops.c +++ b/fs/cifs/smb2ops.c @@ -2485,7 +2485,7 @@ smb3_init_transform_rq(struct TCP_Server_Info *server, struct smb_rqst *new_rq, struct page **pages; struct smb2_transform_hdr *tr_hdr; unsigned int npages = old_rq->rq_npages; - unsigned int orig_len = 0; + unsigned int orig_len; int i; int rc = -ENOMEM; @@ -2499,9 +2499,6 @@ smb3_init_transform_rq(struct TCP_Server_Info *server, struct smb_rqst *new_rq, new_rq->rq_pagesz = old_rq->rq_pagesz; new_rq->rq_tailsz = old_rq->rq_tailsz; - for (i = 0; i < old_rq->rq_nvec; i++) - orig_len += old_rq->rq_iov[i].iov_len; - for (i = 0; i < npages; i++) { pages[i] = alloc_page(GFP_KERNEL|__GFP_HIGHMEM); if (!pages[i]) @@ -2524,6 +2521,8 @@ smb3_init_transform_rq(struct TCP_Server_Info *server, struct smb_rqst *new_rq, if (!tr_hdr) goto err_free_iov; + orig_len = smb2_rqst_len(old_rq, false); + /* fill the 2nd iov with a transform header */ fill_transform_hdr(tr_hdr, orig_len, old_rq); new_rq->rq_iov[0].iov_base = tr_hdr; diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h index 78371c1a6503..3ae208ac2a77 100644 --- a/fs/cifs/smb2proto.h +++ b/fs/cifs/smb2proto.h @@ -113,6 +113,8 @@ extern int smb2_unlock_range(struct cifsFileInfo *cfile, extern int smb2_push_mandatory_locks(struct cifsFileInfo *cfile); extern void smb2_reconnect_server(struct work_struct *work); extern int smb3_crypto_aead_allocate(struct TCP_Server_Info *server); +extern unsigned long +smb2_rqst_len(struct smb_rqst *rqst, bool skip_rfc1002_marker); /* * SMB2 Worker functions - most of protocol specific implementation details diff --git a/fs/cifs/smbdirect.c b/fs/cifs/smbdirect.c index e459c97151b3..a781b4d73990 100644 --- a/fs/cifs/smbdirect.c +++ b/fs/cifs/smbdirect.c @@ -2087,7 +2087,7 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst) struct kvec vec; int nvecs; int size; - unsigned int buflen = 0, remaining_data_length; + unsigned int buflen, remaining_data_length; int start, i, j; int max_iov_size = info->max_send_size - sizeof(struct smbd_data_transfer); @@ -2111,25 +2111,13 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst) log_write(ERR, "expected the pdu length in 1st iov, but got %zu\n", rqst->rq_iov[0].iov_len); return -EINVAL; } - iov = &rqst->rq_iov[1]; - - /* total up iov array first */ - for (i = 0; i < rqst->rq_nvec-1; i++) { - buflen += iov[i].iov_len; - } /* * Add in the page array if there is one. The caller needs to set * rq_tailsz to PAGE_SIZE when the buffer has multiple pages and * ends at page boundary */ - if (rqst->rq_npages) { - if (rqst->rq_npages == 1) - buflen += rqst->rq_tailsz; - else - buflen += rqst->rq_pagesz * (rqst->rq_npages - 1) - - rqst->rq_offset + rqst->rq_tailsz; - } + buflen = smb2_rqst_len(rqst, true); if (buflen + sizeof(struct smbd_data_transfer) > info->max_fragmented_send_size) { @@ -2139,6 +2127,8 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst) goto done; } + iov = &rqst->rq_iov[1]; + 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); diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c index a3ea42a4cb98..fb57dfbfb749 100644 --- a/fs/cifs/transport.c +++ b/fs/cifs/transport.c @@ -201,15 +201,24 @@ smb_send_kvec(struct TCP_Server_Info *server, struct msghdr *smb_msg, return 0; } -static unsigned long -smb2_rqst_len(struct smb_rqst *rqst) +unsigned long +smb2_rqst_len(struct smb_rqst *rqst, bool skip_rfc1002_marker) { unsigned int i; - struct kvec *iov = rqst->rq_iov; + struct kvec *iov; + int nvec; unsigned long buflen = 0; + if (skip_rfc1002_marker && rqst->rq_iov[0].iov_len == 4) { + iov = &rqst->rq_iov[1]; + nvec = rqst->rq_nvec - 1; + } else { + iov = rqst->rq_iov; + nvec = rqst->rq_nvec; + } + /* total up iov array first */ - for (i = 0; i < rqst->rq_nvec; i++) + for (i = 0; i < nvec; i++) buflen += iov[i].iov_len; /* @@ -262,7 +271,7 @@ __smb_send_rqst(struct TCP_Server_Info *server, int num_rqst, (char *)&val, sizeof(val)); for (j = 0; j < num_rqst; j++) - send_length += smb2_rqst_len(&rqst[j]); + send_length += smb2_rqst_len(&rqst[j], true); rfc1002_marker = cpu_to_be32(send_length); /* Generate a rfc1002 marker for SMB2+ */
In smb3_init_transform_rq(), 'orig_len' was only counting the request length, but forgot to count any data pages in the request. Writing or creating files with the 'seal' mount option was broken. In addition, do some code refactoring by exporting smb2_rqst_len() to calculate the appropriate packet size and avoid duplicating the same calculation all over the code. The start of the io vector is either the rfc1002 length (4 bytes) or a SMB2 header which is always > 4. Use this fact to check and skip the rfc1002 length if requested. Signed-off-by: Paulo Alcantara <palcantara@suse.de> --- fs/cifs/smb2ops.c | 7 +++---- fs/cifs/smb2proto.h | 2 ++ fs/cifs/smbdirect.c | 18 ++++-------------- fs/cifs/transport.c | 19 ++++++++++++++----- 4 files changed, 23 insertions(+), 23 deletions(-)