mkdir-dup test flapping

[Jeremy Allison]

On Sat, Mar 12, 2016 at 08:27:11AM +1300, Andrew Bartlett wrote:
> On Thu, 2016-03-10 at 16:49 -0800, Jeremy Allison wrote:
> > On Fri, Mar 11, 2016 at 01:37:17PM +1300, Andrew Bartlett wrote:
> > > 
> > > Isn't there still a race between whatever code first calls stat()
> > > and
> > > fills in smb_dname->st and the fstat() in vfs_stat_fsp()?
> > > 
> > > The directory could have been crated by the time we enter this
> > > function, but not be chowned() until just before vfs_stat_fsp().
> > 
> > There's always the possibility of a race with 2 clients
> > talking to different smbd's.
> > 
> > The code is trying to ensure that after it's done
> > an open(dname, O_DIRECTORY) that the handle is
> > pointing to the same file stat that came into
> > the function - to make sure no one did a directory
> > rename or rmdir/mkdir in between.
> > 
> > We need that so fsp->file_id is correct (and
> > we can detect if the underlying dir changed).
> > fsp is passed into the fd_open() -> open()
> > so we need it to match (a VFS module might
> > use it). Chicken and egg problem.
> > 
> > > Is there anything that checking the file ownership (rather than
> > > checking the IS_DIR and dev/inode) is protecting?  Why do we stat()
> > > this twice in any case?
> > 
> > Actually, checking the file ownership - probably
> > not. The actual open itself is safe enough as underlying
> > file system perms protect us. I remember having check_same_stat()
> > already around so re-used it. check_same_dev_ino() is
> > almost certainly safe enough and is used inside
> > open_file_ntcreate() in the same place.
> 
> With this patch we pass > 10 autobuilds on the Catalyst Cloud, either
> succeeding or failing on later tests (mostly that...).
> 
> Can you look over this carefully for me please?

Yes, I think this is going to work. Reviewed-by: Jeremy Allison <jra at samba.org>

There's no security benefit in the check_same_stat(), so checking
for directory and check_same_dev_ino() is enough.

Pushed - once I goes through I'll cherry-pick for back porting.

Thanks a *LOT* for your help on this !

Cheers,

	Jeremy.