Patchset to add asynchronous open/close to master

Jeremy Allison jra at samba.org
Wed Jun 20 10:24:05 MDT 2012


On Wed, Jun 20, 2012 at 11:41:05AM +0200, Volker Lendecke wrote:
> 
> In a commit you mentioned that it has a race condition
> fixed. Can you explain that? I am very worried that we put
> an open potentially creating a file into a thread and (as it
> seems to me) we fix up potential screwups later with a
> fchown. This to me far too badly smells like a potential
> race condition. The only way I see to correctly do this is
> by a helper process that does our normal become_user and not
> switch back to become_root(). Threads for my taste are too
> dangerous for this.

The open is only done in the thread if the flags are
set to O_CREAT, so we are only creating a file.
I cache the current euid and guid at the time the
VFS_OPEN is invoked, so we know who should own the
file.

In addition, an idea from a conversation I had with
you means I add in O_EXCL into the flags - even if
the client did not specify this. I save off the
initial flags the client requested.

So the open inside the thread can only create a file,
never open an existing file. If the open fails with
EEXIST, then we know we were raced, and so once
the thread terminates and the open call is rescheduled
I retry the open synchronously using the default path. 

If the open succeeded, we *know* we created the file.
So once the thread terminates I do an fstat() on the
returned fd and look at the created uid/gid.

If smbd did not change uids (the normal case) then
we are done. If it did and the threaded open hit
the race condition, then the uid on the file
will be different. If so, then we need to fix up
using the fchown call.

There is a slight complication where the parent
directory has the SETGID bit set so the owning
gid needs to be the parent directory gid, not
the gid from the process token, but there is
code to look at and handle that in the module.

Hope that explains things. As you know, the
open path emulating an CreateFile always has
some races due to there being no kernel-level
CreateFile call and emulated primitive not being
atomic.

I'm satisfied with this code coping with the
issues correctly, so I'd like you to trust
me on this, if you're comfortable with this
explaination.

Thanks,

	Jeremy. 


More information about the samba-technical mailing list