[PATCHES] Fix smbd open race and render defer_open test less flakey

Michael Adam obnox at samba.org
Thu Sep 11 12:07:13 MDT 2014


On 2014-09-11 at 19:56 +0200, Michael Adam wrote:
> Jeremy,
> 
> On 2014-09-11 at 10:06 -0700, Jeremy Allison wrote:
> > On Thu, Sep 11, 2014 at 06:04:54PM +0200, Michael Adam wrote:
> > > On 2014-09-10 at 10:21 -0700, Jeremy Allison wrote:
> > > > On Wed, Sep 10, 2014 at 01:57:44PM +0200, Michael Adam wrote:
> > > > > 
> > > > > The first is due to a race in open_file() which I
> > > > > fixed in the other attached patch: The file vanished
> > > > > between check for existence and the acl check.
> > > > > ==> review/push/comments appreciated.
> > > > 
> > > > Great catch Michael
> > > 
> > > Gosh! After anoter > 1600 runs, defer_open failed for me again
> > > and showed that the patch may have been too short-sighted:
> > > 
> > > In open_file_ntcreate() we do this:
> > > 
> > > 	file_existed = smb_fname->st;
> > > 	...
> > > 	open_file(...);
> > > 	...
> > > 	if (file_existed &&
> > > 	    !check_same_dev_ino(&saved_stat, &smb_fname->st))
> > > 	{
> > > 		close;
> > > 		fail with NT_STATUS_ACCESS_DENIED;
> > > 	}
> > > 
> > > So I nicely fixed open_file, but the caller stumbles. ;-)
> > 
> > This is a race that's been there forever I think.
> > 
> > We can make the existence->usage race smaller, but
> > never eliminate it completely (and in most cases
> > it really doesn't matter).
> 
> Right, we only want to improve on the case
> where we ourselves have created the file (again).

And as I just learned from my test-loop, the
case where another smbd also creates the file
in the race is of course still there, and that
can still let the defer_open test fail...

Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140911/270e8cc9/attachment.pgp>


More information about the samba-technical mailing list