mkdir-dup test flapping

[Jeremy Allison]

On Tue, Mar 08, 2016 at 08:54:32PM -0800, Jeremy Allison wrote:
> On Wed, Mar 09, 2016 at 04:11:05PM +1300, Douglas Bagnall wrote:
> > We looked at this some more, and Andrew seemed to understand and wrote
> > the attached patch.
> > 
> > We need to decide if the change in owner is enough reason to refuse
> > the open(), or if we should just continue.
> > 
> > On the basis that the owner change is not significant, we reworked the
> > code to do the lstat() after the fstat(), and not compare the owner.
> 
> Fix is really close, but still needs a little work -
> the call to:
> 
> fsp->file_id = vfs_file_id_from_sbuf(conn, &smb_dname->st);
> 
> needs to be moved too (as in the race condition
> when need_lstat is true smb_dname->st isn't correctly
> initialized yet), or the lstat needs to be done both
> before and after. Doing the lstat twice might be
> safer as I'll need to check what uses fsp->file_id
> to ensure moving to after the lstat is safe.
> 
> *Really* nice work though - thanks ! It's late
> here now so I'll take a look at this first thing
> in thr morning (US Pacific time) and post an
> updated patch for you to look at tomorrow.

OK - here it is. I looked over this very carefully
and this is what I came up with.

There's no security reason for re-doing the
lstat after the vfs_fsp_stat() call as if the
owner and SD were changed, any operation on
the directory is still protected by the new
owner/ACL that was placed upon the directory.

Anyone with permission to do those changes
can do them whether we have the directory
open or not.

For paranoia's sake I did add an extra ISDIR
check once we (should) have a file handle to
make sure the pathname the fsp is pointing to
is a directory when exiting the function
(or course, as with all races there's
nothing to stop a properly credentialed
used renaming/replacing that pathname
later, but that's what a fileserver is
designed to do :-).

I created bug https://bugzilla.samba.org/show_bug.cgi?id=11780
to track this and get it back-ported to
release brances.

Let me know what you think !

Jeremy.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-s3-smbd-Fix-error-in-mkdir-race-condition-detection.patch
Type: text/x-diff
Size: 3274 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160309/0663bc8b/0001-s3-smbd-Fix-error-in-mkdir-race-condition-detection.diff>