| Message ID | 20191121193514.3086-3-pshilov@microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] CIFS: Close open handle after interrupted close | expand |
Very nice. Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com > On Fri, Nov 22, 2019 at 5:35 AM Pavel Shilovsky <piastryyy@gmail.com> wrote: > > When an OPEN command is cancelled we mark a mid as > cancelled and let the demultiplex thread process it > by closing an open handle. The problem is there is > a race between a system call thread and the demultiplex > thread and there may be a situation when the mid has > been already processed before it is set as cancelled. > > Fix this by processing cancelled requests when mids > are being destroyed which means that there is only > one thread referencing a particular mid. Also set > mids as cancelled unconditionally on their state. > > Cc: Stable <stable@vger.kernel.org> > Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> > --- > fs/cifs/connect.c | 6 ------ > fs/cifs/transport.c | 10 ++++++++-- > 2 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > index e63d16d8048a..59feb2de389e 100644 > --- a/fs/cifs/connect.c > +++ b/fs/cifs/connect.c > @@ -1229,12 +1229,6 @@ cifs_demultiplex_thread(void *p) > for (i = 0; i < num_mids; i++) { > if (mids[i] != NULL) { > mids[i]->resp_buf_size = server->pdu_size; > - if ((mids[i]->mid_flags & MID_WAIT_CANCELLED) && > - mids[i]->mid_state == MID_RESPONSE_RECEIVED && > - server->ops->handle_cancelled_mid) > - server->ops->handle_cancelled_mid( > - mids[i]->resp_buf, > - server); > > if (!mids[i]->multiRsp || mids[i]->multiEnd) > mids[i]->callback(mids[i]); > diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c > index bb52751ba783..987ffcd5ca3a 100644 > --- a/fs/cifs/transport.c > +++ b/fs/cifs/transport.c > @@ -93,8 +93,14 @@ static void _cifs_mid_q_entry_release(struct kref *refcount) > __u16 smb_cmd = le16_to_cpu(midEntry->command); > unsigned long now; > unsigned long roundtrip_time; > - struct TCP_Server_Info *server = midEntry->server; > #endif > + struct TCP_Server_Info *server = midEntry->server; > + > + if (midEntry->resp_buf && (midEntry->mid_flags & MID_WAIT_CANCELLED) && > + midEntry->mid_state == MID_RESPONSE_RECEIVED && > + server->ops->handle_cancelled_mid) > + server->ops->handle_cancelled_mid(midEntry->resp_buf, server); > + > midEntry->mid_state = MID_FREE; > atomic_dec(&midCount); > if (midEntry->large_buf) > @@ -1115,8 +1121,8 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses, > midQ[i]->mid, le16_to_cpu(midQ[i]->command)); > send_cancel(server, &rqst[i], midQ[i]); > spin_lock(&GlobalMid_Lock); > + midQ[i]->mid_flags |= MID_WAIT_CANCELLED; > if (midQ[i]->mid_state == MID_REQUEST_SUBMITTED) { > - midQ[i]->mid_flags |= MID_WAIT_CANCELLED; > midQ[i]->callback = cifs_cancelled_callback; > cancelled_mid[i] = true; > credits[i].value = 0; > -- > 2.17.1 >
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index e63d16d8048a..59feb2de389e 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -1229,12 +1229,6 @@ cifs_demultiplex_thread(void *p) for (i = 0; i < num_mids; i++) { if (mids[i] != NULL) { mids[i]->resp_buf_size = server->pdu_size; - if ((mids[i]->mid_flags & MID_WAIT_CANCELLED) && - mids[i]->mid_state == MID_RESPONSE_RECEIVED && - server->ops->handle_cancelled_mid) - server->ops->handle_cancelled_mid( - mids[i]->resp_buf, - server); if (!mids[i]->multiRsp || mids[i]->multiEnd) mids[i]->callback(mids[i]); diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c index bb52751ba783..987ffcd5ca3a 100644 --- a/fs/cifs/transport.c +++ b/fs/cifs/transport.c @@ -93,8 +93,14 @@ static void _cifs_mid_q_entry_release(struct kref *refcount) __u16 smb_cmd = le16_to_cpu(midEntry->command); unsigned long now; unsigned long roundtrip_time; - struct TCP_Server_Info *server = midEntry->server; #endif + struct TCP_Server_Info *server = midEntry->server; + + if (midEntry->resp_buf && (midEntry->mid_flags & MID_WAIT_CANCELLED) && + midEntry->mid_state == MID_RESPONSE_RECEIVED && + server->ops->handle_cancelled_mid) + server->ops->handle_cancelled_mid(midEntry->resp_buf, server); + midEntry->mid_state = MID_FREE; atomic_dec(&midCount); if (midEntry->large_buf) @@ -1115,8 +1121,8 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses, midQ[i]->mid, le16_to_cpu(midQ[i]->command)); send_cancel(server, &rqst[i], midQ[i]); spin_lock(&GlobalMid_Lock); + midQ[i]->mid_flags |= MID_WAIT_CANCELLED; if (midQ[i]->mid_state == MID_REQUEST_SUBMITTED) { - midQ[i]->mid_flags |= MID_WAIT_CANCELLED; midQ[i]->callback = cifs_cancelled_callback; cancelled_mid[i] = true; credits[i].value = 0;
When an OPEN command is cancelled we mark a mid as cancelled and let the demultiplex thread process it by closing an open handle. The problem is there is a race between a system call thread and the demultiplex thread and there may be a situation when the mid has been already processed before it is set as cancelled. Fix this by processing cancelled requests when mids are being destroyed which means that there is only one thread referencing a particular mid. Also set mids as cancelled unconditionally on their state. Cc: Stable <stable@vger.kernel.org> Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> --- fs/cifs/connect.c | 6 ------ fs/cifs/transport.c | 10 ++++++++-- 2 files changed, 8 insertions(+), 8 deletions(-)