Re: [PATCH] nfs_unlink() race (was: nfs_refresh_inode: inode number mismatch)

[Trond Myklebust <trond.myklebust@fys.uio.no>]

>>>>> " " == viro  <viro@parcelfarce.linux.theplanet.co.uk> writes:

     > Aliasing could be dealt with.  They would have the same inode,
     > so it's easy to detect.

I suppose we could... Is the procedure that nfs_lookup() should first
rehash the dropped dentry, then return it instead of NULL?

     > The real problem is different: what happens if I take
     > silly-renamed file and rename it away?  You suddenly get ->dir
     > and ->dentry if your nfs_unlinkdata having nothing to do with
     > each other.

->dir is the important one here, as it provides the filehandle. We
only use ->dentry in order to give us a name/string for the
NFSPROC_REMOVE call.

     > AFAICS, dcache will not get into inconsistent state, but it
     > will have very little to do with state of server...

But that's the best we can do in any scenario. NFS does not ever give
you a guarantee that someone won't screw things up on the server, nor
does it give you any failsafe mechanism for detecting it.
That's why operation atomicity is such an issue with the current
kernel, and is why I'm so eager to push the intent patches into 2.6.

Sure sillyrename fails miserably in the atomicity department. There's
bugger all we can do about that: if you are suggesting we should rely
on some sort of revalidation before we unlink, then that's just as
race prone as what we have now.

Cheers,
  Trond