[PATCH] Version 2 - Fix smbd behavior with SMB2-FLUSH on a directory handle.

Jeremy Allison jra at samba.org
Fri May 11 15:24:04 UTC 2018


On Fri, May 11, 2018 at 09:17:47AM +0200, Ralph Böhme wrote:
> 
> oh, good catch. How did you find this?

Steve trying to get databases to work over cifsfs to
Samba :-).

> Is the access check below really correct? See below.
> 
> >  	if (!CHECK_WRITE(fsp)) {
> > -		tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED);
> > -		return tevent_req_post(req, ev);
> > +		if (!fsp->is_directory) {
> > +			tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED);
> > +			return tevent_req_post(req, ev);
> > +		}
> > +
> > +		/*
> > +		 * Directories are not writable in the conventional
> > +		 * sense, but if opened with
> > +		 * FILE_ADD_FILE|FILE_ADD_SUBDIRECTORY
> > +		 * they can be flushed.
> > +		 */
> > +		if ((fsp->access_mask &
> > +				(FILE_ADD_FILE|FILE_ADD_SUBDIRECTORY))==0) {
> > +			tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED);
> > +			return tevent_req_post(req, ev);
> > +		}
> 
> if I understand you right, the client must have *both* FILE_ADD_FILE *and*
> FILE_ADD_SUBDIRECTORY, so the check should be:

I think it's *either* FILE_ADD_FILE or FILE_ADD_SUBDIRECTORY, but
I'll extend the torture test to make sure.

> > +		if ((fsp->access_mask &
> > +				(FILE_ADD_FILE|FILE_ADD_SUBDIRECTORY))==(FILE_ADD_FILE|FILE_ADD_SUBDIRECTORY)) {
> > +			tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED);
> > +			return tevent_req_post(req, ev);
> > +		}
> 
> Either way, you'd make me a happy man if you could rewrite the if condition to
> something that resembles my attached FIXUP. Thanks! :)

I had a hard time with the if condition :-). I'll
post an updated patch once I know if it's both or
OR on the permission bits.

Thanks for the review !

Jeremy.



More information about the samba-technical mailing list