вт, 28 авг. 2018 г. в 19:19, Ronnie Sahlberg <lsahlber@xxxxxxxxxx>:
>
> For compounded PDUs we whould only wake the waiting thread for the
> very last PDU of the compound.
> We do this so that we are guaranteed that the demultiplex_thread will
> not process or access any of those MIDs any more once the send/recv
> thread starts processing.
>
> Else there is a race where at the end of the send/recv processing we
> will try to delete all the mids of the compound. If the multiplex
> thread still has other mids to process at this point for this compound
> this can lead to an oops.
>
> Signed-off-by: Ronnie Sahlberg <lsahlber@xxxxxxxxxx>
> ---
> fs/cifs/transport.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index 9e3a1e87ae1c..49a800ce75d6 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -773,6 +773,11 @@ cifs_setup_request(struct cifs_ses *ses, struct smb_rqst *rqst)
> return mid;
> }
>
> +static void
> +cifs_noop_callback(struct mid_q_entry *mid)
> +{
> +}
> +
> int
> compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
> const int flags, const int num_rqst, struct smb_rqst *rqst,
> @@ -826,9 +831,15 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
> return PTR_ERR(midQ[i]);
> }
>
> + /*
> + * We don't invoke the callback compounds unless it is the last
> + * request.
> + */
> midQ[i]->mid_state = MID_REQUEST_SUBMITTED;
> + if (ses->server->ops->next_header &&
> + ses->server->ops->next_header(rqst[i].rq_iov[0].iov_base))
> + midQ[i]->callback = cifs_noop_callback;
Wouldn't it be easier to just check for i < num_rqst - 1 ? Having
ses->server->ops->next_header(rqst[i].rq_iov[0].iov_base) return false
should be a bug here. SMB1 code doesn't support compounding at all, so
num_rqst is always 1.
> }
> -
> cifs_in_send_inc(ses->server);
> rc = smb_send_rqst(ses->server, num_rqst, rqst, flags);
> cifs_in_send_dec(ses->server);
> @@ -909,6 +920,12 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
> midQ[i]->resp_buf = NULL;
> }
> out:
> + /*
> + * This will dequeue all mids. After this it is important that the
> + * demultiplex_thread will not process any of these mids any futher.
> + * This is prevented above by using a noop callback that will not
> + * wake this thread except for the very last PDU.
> + */
> for (i = 0; i < num_rqst; i++)
> cifs_delete_mid(midQ[i]);
> add_credits(ses->server, credits, optype);
> --
> 2.13.3
>
--
Best regards,
Pavel Shilovsky
