lkml.org 
[lkml]   [2010]   [Jun]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [V9fs-developer] [GIT PULL] 9p file system bug fixes for 2.6.35-rc2
    Linus Torvalds wrote:
    >
    > On Mon, 7 Jun 2010, Eric Van Hensbergen wrote:
    >> jvrao (2):
    >> Add a helper function to get fsgid for a file create.
    >> 9p: Add a wstat after TCREATE to fix the gid.
    >
    > Quite frankly, this looks rather broken.
    >
    > It uses "dentry->d_parent" without locking (it so happens to likely be ok,
    > since we are in "create()" and thus should be holding the parent
    > semaphore). On its own, that might be excusable (if people were even
    > _aware_ of the this locking rule!), but it does so just to get the inode
    > pointer to that parent.
    >
    > And the only thing that makes it ok to access dentry->d_parent - the fact
    > that we are in v9fs_create() - is also the thing that should have made
    > people look at the arguments to the function and say "hmm".

    Silly me. I sent out another patch using the dir inode passed through arguments.
    But we still need to analyze the use of dentry->d_parent in other parts of code..

    - JV


    >
    > We pass in the directory inode pointer as an argument to the create
    > function! The code could have used that thing directly, instead of
    > mucking around with dentry pointers that it had no business looking at.
    >
    > I see why it seems to have happened: v9fs does the exact same thing for
    > the pre-existing "v9fs_fid_lookup()". So there is history to this
    > behavior.
    >
    > Maybe people weren't aware of the fact that just dereferencing
    > dentry->d_parent willy-nilly isn't actually allowed. That field changes.
    > Sure, there are cases where it's ok, but this is a dangerous thing to do
    > in general.
    >
    > In fact, the other thing that I find doing that whole "dentry->d_parent"
    > thing seems to literally be broken. If you look at v9fs_fid_lookup(),
    > you'll notice how it walks up the d_parent chain, and at that point you do
    > NOT own the directory i_mutex, so at that point d_parent really _can_ be
    > changing wildly due to concurrent renames or whatever.
    >
    > So 9pfs seems to have some preexisting bugs in this area. I'm not going to
    > pull new bug-prone code. See the other discussions about being tight this
    > release about really _only_ taking regressions after the merge window
    > closed.
    >
    > Linus
    >
    > ------------------------------------------------------------------------------
    > ThinkGeek and WIRED's GeekDad team up for the Ultimate
    > GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the
    > lucky parental unit. See the prize list and enter to win:
    > http://p.sf.net/sfu/thinkgeek-promo
    > _______________________________________________
    > V9fs-developer mailing list
    > V9fs-developer@lists.sourceforge.net
    > https://lists.sourceforge.net/lists/listinfo/v9fs-developer




    \
     
     \ /
      Last update: 2010-06-08 16:33    [W:0.024 / U:1.036 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site