lkml.org 
[lkml]   [2003]   [Jun]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] nfs_unlink() race (was: nfs_refresh_inode: inode number mismatch)
    On Mon, Jun 09, 2003 at 06:51:41AM -0700, Frank Cusack wrote:

    > When foo is unlinked, nfs_unlink() does a sillyrename, this puts the
    > dentry on nfs_delete_queue, and (in the VFS) unhashes it from the dcache.
    > This causes a problem, because dentry->d_parent->d_inode is now guaranteed
    > to remain stale. (OK, I'm not really sure about this last part.)

    ????

    What does hashed state have to ->d_parent?

    > Then readdir() returns the new .nfs file, this creates a NEW dentry
    > (we just moved the first one to nfs_delete_queue and did not create a
    > negative dentry) which now has d_count==1 so instead of sillyrename we
    > just remove it (but note, we actually have this file open). Then rmdir
    > succeeeds.
    >
    > Now, we have a dentry on nfs_delete_queue which a) has already been
    > unlinked and b) whose dentry->d_parent DNE and dentry->d_parent->d_inode
    > DNE. Of course this will cause confusion later!

    b) is bogus. Unhashing does nothing to ->d_parent.

    > Note that if a process does a drive by on the .nfs file (another round
    > of unlinked-but-open) before we unlink it, we would sillyrename it again.
    > We'd now have two different dentry's on the delete queue for the same
    > file. (One of them would just leak, I think--possible local DoS?)

    Two different dentries for the same file is obviously not a problem...

    > 1) Don't unhash the dentry after silly-renaming. In 2.2, each fs is
    > responsible for doing a d_delete(), in 2.4 it happens in the VFS and
    > I think it was just an oversight that the 2.4 VFS doesn't consider
    > sillyrename (considering the code and comments that are cruft).
    >
    > This preserves the unlinked-but-open semantic, but breaks rmdir. So
    > it's not a clear winner from a semantics POV. dentry->d_count is
    > always correct, which sounds like a plus.
    >
    > The patch to make this work is utterly simple, which is a big plus.

    ... and AFAICS it opens a huge can of worms with races in NFS unlink/rename.

    Sigh... I'll look through that code and try to reconstruct the analysis.
    It used to be a very big mess and there was fairly subtle logics around
    unhashing/checks for d_count/etc. involved in fixing ;-/ And there was
    a lot of changes since then. Oh, well...
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 13:36    [W:2.921 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site