Thanks. I will have a look and merge your changes.
I have reworked the patches quite a bit and will re-send the patch series in a few days.
----- Original Message -----
From: "Aurélien Aptel" <aaptel@xxxxxxxx>
To: "Ronnie Sahlberg" <lsahlber@xxxxxxxxxx>, "linux-cifs" <linux-cifs@xxxxxxxxxxxxxxx>, "Long Li" <longli@xxxxxxxxxxxxx>
Cc: "Steve French" <smfrench@xxxxxxxxx>
Sent: Thursday, 1 March, 2018 1:21:43 AM
Subject: Re: [PATCH 00/14] cifs: add compounding support for smb2+
Hi Ronnie, all,
I've rebasing this on current for-next since it couldn't be applied
anymore. Beside the simple conflicts there are a couple of non-trivial
changes I've made.
Since some SMB DIRECT code made it in the meantime I had to handle some
conflicts.
in smb2pdu.c, smb2_async_writev():
> rqst.rq_iov = iov;
> rqst.rq_nvec = 2;
> rqst.rq_pages = wdata->pages;
> rqst.rq_npages = wdata->nr_pages;
> rqst.rq_pagesz = wdata->pagesz;
> rqst.rq_tailsz = wdata->tailsz;
> #ifdef CONFIG_CIFS_SMB_DIRECT
> if (wdata->mr) {
> iov[0].iov_len += sizeof(struct smbd_buffer_descriptor_v1);
> rqst.rq_npages = 0;
> }
> #endif
since iov is an array of size 1, I've changed the SMB_DIRECT code to use
iov[0] instead of iov[1]. But I have no idea if it's really correct.
The thing that bothers me is iov[] has a size of 1 yet
> rqst.rq_iov = iov;
> rqst.rq_nvec = 2;
mhh..
=========
The other thing I changed was moving the SMB311 preauth hash update to the new
cifs_send_receive() wrapper like so:
> int
> cifs_send_recv(const unsigned int xid, struct cifs_ses *ses,
> struct smb_rqst *rqst, int *resp_buf_type, const int flags,
> struct kvec *resp_iov)
> {
> int rc;
>
> #ifdef CONFIG_CIFS_SMB311
> if (ses->status == CifsNew)
> smb311_update_preauth_hash(ses, rqst->rq_iov,
> rqst->rq_nvec);
> #endif
>
> rc = compound_send_recv(xid, ses, flags, 1, rqst, resp_buf_type,
> resp_iov);
>
> #ifdef CONFIG_CIFS_SMB311
> if (ses->status == CifsNew) {
> smb311_update_preauth_hash(ses, resp_iov, 1);
> }
> #endif
> return rc;
>
=========
Small change for a sparse warning I was getting:
> #define COMPOUND_FID 0xFFFFFFFFFFFFFFFFULL
Added the ULL suffix as the constant is too big for an int.
=========
Now these I have left these as-is but I have some comments:
=========
in transport.c, compound_send_receive():
> for (i = 0; i < num_rqst; i++) {
> cifs_save_when_sent(midQ[i]);
>
> if (rc < 0)
> ses->server->sequence_number -= 2;
> mutex_unlock(&ses->server->srv_mutex);
We potentially unlock the mutex at every loop. Is that on purpose?
=========
In cifs_demultiplex_thread():
> pdu_length = get_rfc1002_length(buf);
> - server->total_size = pdu_length;
>
> - cifs_dbg(FYI, "RFC1002 header 0x%x\n", pdu_length);
> + cifs_dbg(FYI, "RFC1002 header 0x%x\n", server->total_size);
> if (!is_smb_response(server, buf[0]))
> continue;
> +next_pdu:
> + server->total_size = pdu_length;
You set server->total_size *after* printing it.
=========
After the rebase I've tried testing mounting a WS2016 share and listing
the root. If the correct files are listed its OK anything else is ERR:
vers=1.0 ... OK
vers=2.0 ... OK
vers=2.0,sec=ntlmsspi ... ERR
vers=2.1 ... OK
vers=2.1,sec=ntlmsspi ... ERR
vers=3.0 ... OK
vers=3.0,sec=ntlmsspi ... ERR
vers=3.0,seal ... OK
vers=3.0,seal,sec=ntlmsspi ... OK
vers=3.11 ... ERR
vers=3.11,sec=ntlmsspi ... ERR
vers=3.11,seal ... ERR
vers=3.11,seal,sec=ntlmsspi ... ERR
Most of the ntlmsspi errors (thats when we require signing) are due to
bad signing on certain packets (ones triggered by ls):
Find Request File: SMB2_FIND_ID_FULL_DIRECTORY_INFO Pattern: *
Find Response, Error: STATUS_ACCESS_DENIED
(if access is ok without signing -> error due to wrong sig)
Weirdly enough, encryption+signingon 3.0 works.
As for SMB311, mount fails and I see lengths issues
> fs/cifs/transport.c: Sending smb: smb_len=170
> fs/cifs/connect.c: RFC1002 header 0x0
> fs/cifs/smb2misc.c: SMB2 data length 120 offset 128
> fs/cifs/smb2misc.c: SMB2 len 248
> fs/cifs/smb2misc.c: Calculated size 248 length 308 mismatch mid 0
> SMB2 server sent bad RFC1001 len 308 not 248
And it does weird things from there.
I can provide full kernel debug log and network captures each of the
run.
I've attached my rebased patches (I've added my sign-off to patches I
had to modify to apply or added changes to).
Cheers,
--
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)
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
