[PATCH] Fix bug 9722 - Samba does not properly handle Oplock breaks in compound requests

[Jeremy Allison]

On Sat, May 04, 2013 at 08:14:53PM -0700, Richard Sharpe wrote:
> 
> Is there a problem here?
> 
> diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
> index 57e9c7b..9a55d6a 100644
> --- a/source3/smbd/smb2_server.c
> +++ b/source3/smbd/smb2_server.c
> @@ -1599,6 +1599,14 @@ static NTSTATUS smbd_smb2_request_process_cancel(struct s
> mbd_smb2_request *req)
>  		uint64_t message_id;
>  		uint64_t async_id;
> 
> +		if (cur->compound_related) {
> +			/*
> +			 * Never cancel anything in a compound request.
> +			 * Way too hard to deal with the result.
> +			 */
> +			continue;
> +		}
> +
>  		outhdr = SMBD_SMB2_OUT_HDR_PTR(cur);
> 
>  		message_id = BVAL(outhdr, SMB2_HDR_MESSAGE_ID);
> -- 
> 1.8.1.2
> 
> because just before the for loop that this goes in, we do this:
> 
>         /*
>          * we don't need the request anymore
>          * cancel requests never have a response
>          */
>         DLIST_REMOVE(req->sconn->smb2.requests, req);
>         TALLOC_FREE(req);
> 
> which seems to suggest that the request is now gone from our view.

No, this isn't a problem. The DLIST_REMOVE and TALLOC_FREE
are being done on 'req', which in this case is the SMB2_OP_CANCEL
request, containing a MID-to-be-cancelled message id.

The for loop is looking at the smb2 req's currently being
processed, to see if they are cancellable (we have to match
the message id and there has to be a cancel function
availabel), so the additional code just ignores cancel
requests on any current (the variable cur iterates over
all the current reqs in the 'being processed' list) requests
that are marked as compound related.

It's always ok to ignore a cancel request, as such a
thing is inherently racy (although we should always
try and honor a cancel on a long-lived op like notify,
but such ops are almost always sent either in non-compound
requests, or at the end of a compound chain in which
case they're split off into non-compound requests once
the previous compound elements are finished).

Hope this explains it !

Cheers,

	Jeremy.