merged this patch in with the one it fixes and pushed to cifs-2.6.git for-next Running buildbot on them now: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/509 On Tue, Feb 23, 2021 at 3:49 AM Rohith <rohiths.msft@xxxxxxxxx> wrote: > > Hi Steve, > > > > Please find the attached patch which will fix the regression caused by previous patch. > > During read path when pdu length is greater than CIFSMaxBufSize, “bufs” pointer is not updated during “receive_transform”. So, oops occurred while accessing “bufs” pointer. > > > > I ran cifs/100 test on my local machine: > > dd if=/dev/zero of=$TEST_DIR/$$/big-file bs=4M count=1 > dd if=$TEST_DIR/$$/big-file of=/dev/null bs=4M count=1 > > > > > > Regards, > > Rohith > > > > Sent from Mail for Windows 10 > > > > From: Shyam Prasad N > Sent: Monday, February 22, 2021 10:53 PM > To: Pavel Shilovsky > Cc: Rohith; Steve French; linux-cifs@xxxxxxxxxxxxxxx; sribhat.msa@xxxxxxxxxxx > Subject: Re: TCON reconnect during STATUS_NETWORK_NAME_DELETED > > > > Looks good to me. > > Reviewed-by: Shyam Prasad N <sprasad@xxxxxxxxxxxxx> > > > > On Mon, Feb 22, 2021 at 9:21 AM Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote: > > > > > > Both patches look good. Thanks! > > > > > > Reviewed-by: Pavel Shilovsky <pshilov@xxxxxxxxxxxxx> > > > -- > > > Best regards, > > > Pavel Shilovsky > > > > > > вс, 21 февр. 2021 г. в 22:44, Rohith <rohiths.msft@xxxxxxxxx>: > > > > > > > > Thanks Steve. Will make sure to run checkpatch for further patches. > > > > > > > > > > > > > > > > Regards, > > > > > > > > Rohith > > > > > > > > > > > > > > > > Sent from Mail for Windows 10 > > > > > > > > > > > > > > > > From: Steve French > > > > Sent: Sunday, February 21, 2021 6:26 AM > > > > To: Pavel Shilovsky > > > > Cc: Rohith; linux-cifs@xxxxxxxxxxxxxxx; Shyam Prasad N; sribhat.msa@xxxxxxxxxxx > > > > Subject: Re: TCON reconnect during STATUS_NETWORK_NAME_DELETED > > > > > > > > > > > > > > > > The remaining suggestion of Pavel's is included in this trivial cleanup. > > > > > > > > > > > > > > > > Trivial change to clarify code in smb2_is_network_name_deleted > > > > > > > > Suggested-by: Pavel Shilovsky <pshilov@xxxxxxxxxxxxx> > > > > Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx> > > > > --- > > > > fs/cifs/smb2ops.c | 29 +++++++++++++++-------------- > > > > 1 file changed, 15 insertions(+), 14 deletions(-) > > > > > > > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > > > > index aee9da88a333..b2191babce26 100644 > > > > --- a/fs/cifs/smb2ops.c > > > > +++ b/fs/cifs/smb2ops.c > > > > @@ -2459,23 +2459,24 @@ smb2_is_network_name_deleted(char *buf, struct TCP_Server_Info *server) > > > > struct cifs_ses *ses; > > > > struct cifs_tcon *tcon; > > > > > > > > - if (shdr->Status == STATUS_NETWORK_NAME_DELETED) { > > > > - spin_lock(&cifs_tcp_ses_lock); > > > > - list_for_each(tmp, &server->smb_ses_list) { > > > > - ses = list_entry(tmp, struct cifs_ses, smb_ses_list); > > > > - list_for_each(tmp1, &ses->tcon_list) { > > > > - tcon = list_entry(tmp1, struct cifs_tcon, tcon_list); > > > > - if (tcon->tid == shdr->TreeId) { > > > > - tcon->need_reconnect = true; > > > > - spin_unlock(&cifs_tcp_ses_lock); > > > > - pr_warn_once("Server share %s deleted.\n", > > > > - tcon->treeName); > > > > - return; > > > > - } > > > > + if (shdr->Status != STATUS_NETWORK_NAME_DELETED) > > > > + return; > > > > + > > > > + spin_lock(&cifs_tcp_ses_lock); > > > > + list_for_each(tmp, &server->smb_ses_list) { > > > > + ses = list_entry(tmp, struct cifs_ses, smb_ses_list); > > > > + list_for_each(tmp1, &ses->tcon_list) { > > > > + tcon = list_entry(tmp1, struct cifs_tcon, tcon_list); > > > > + if (tcon->tid == shdr->TreeId) { > > > > + tcon->need_reconnect = true; > > > > + spin_unlock(&cifs_tcp_ses_lock); > > > > + pr_warn_once("Server share %s deleted.\n", > > > > + tcon->treeName); > > > > + return; > > > > } > > > > } > > > > - spin_unlock(&cifs_tcp_ses_lock); > > > > } > > > > + spin_unlock(&cifs_tcp_ses_lock); > > > > } > > > > > > > > static int > > > > > > > > > > > > > > > > On Sat, Feb 20, 2021 at 5:38 PM Steve French <smfrench@xxxxxxxxx> wrote: > > > > > > > > I corrected various checkpatch errors - including two of those Pavel noted, and tentatively merged into cifs-2.6.git for-next. See attached updated patch. > > > > > > > > > > > > > > > > Will fix the indentation next. > > > > > > > > > > > > > > > > On Sat, Feb 20, 2021 at 12:18 PM Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote: > > > > > > > > чт, 18 февр. 2021 г. в 08:10, Rohith <rohiths.msft@xxxxxxxxx>: > > > > > > > > > > Hi Pavel, > > > > > > > > > > > > > > > > > > > > Addressed review comments. Can you please take a look. > > > > > > > > > > > > > > > > > > > > >> Btw, I think is_status_io timeout should be called in this loop for > > > > > > > > > > >> every mid not outside the loop (sorry, missed that in the original > > > > > > > > > > >> review). Could you please fix that separately? > > > > > > > > > > > > > > > > > > > > Yes, will send out different patch for status_io_timeout. > > > > > > > > Thanks! > > > > > > > > The patch looks good. Please find my minor comments below: > > > > > > > > 1. > > > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > > > > index 10fe6d6d2dee..b2f51546c2ef 100644 > > > > --- a/fs/cifs/connect.c > > > > +++ b/fs/cifs/connect.c > > > > @@ -993,6 +993,10 @@ cifs_demultiplex_thread(void *p) > > > > if (mids[i] != NULL) { > > > > mids[i]->resp_buf_size = server->pdu_size; > > > > > > > > + if (server->ops->is_network_name_deleted) { > > > > + server->ops->is_network_name_deleted(bufs[i], > > > > + server); > > > > + } > > > > > > > > ^^^ > > > > remove extra {} > > > > > > > > 2. > > > > +static void > > > > +smb2_is_network_name_deleted(char *buf, struct TCP_Server_Info *server) > > > > +{ > > > > + struct smb2_sync_hdr *shdr = (struct smb2_sync_hdr *)buf; > > > > > > > > + struct list_head *tmp, *tmp1; > > > > + struct cifs_ses *ses; > > > > + struct cifs_tcon *tcon; > > > > + > > > > + if (shdr->Status == STATUS_NETWORK_NAME_DELETED) { > > > > > > > > ^^^ > > > > let's do the opposite check: if status is not > > > > STATUS_NETWORK_NAME_DELETED then return immediately from the function. > > > > This will remove overall indentation for the nested for loops. > > > > > > > > 3. > > > > @@ -4605,6 +4632,10 @@ static void smb2_decrypt_offload(struct > > > > work_struct *work) > > > > #ifdef CONFIG_CIFS_STATS2 > > > > mid->when_received = jiffies; > > > > #endif > > > > + if (dw->server->ops->is_network_name_deleted) { > > > > + dw->server->ops->is_network_name_deleted(dw->buf, > > > > + dw->server); > > > > + } > > > > > > > > ^^^ > > > > remove extra {} > > > > > > > > -- > > > > Best regards, > > > > Pavel Shilovsky > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > Thanks, > > > > > > > > Steve > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > Thanks, > > > > > > > > Steve > > > > > > > > > > > > > > > > -- > > Regards, > > Shyam > > -- Thanks, Steve
