Re: [PATCH 01/46] Revert "fs: use RCU read side protection in d_validate"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Dec 08, 2010 at 12:16:56PM +1100, Dave Chinner wrote:
> On Sat, Nov 27, 2010 at 08:56:03PM +1100, Nick Piggin wrote:
> > This reverts commit 3825bdb7ed920845961f32f364454bee5f469abb.
> > 
> > Patch is broken, you can't dget() without holding any locks!
> 
> I believe you can - for the same reasons we can take a reference to
> an inode without holding the inode_lock. That is, as long as the
> caller already holds an active reference to the dentry,
> dget() can be used to take another reference without needing the
> dcache_lock.
> 
> Such usage appears to be described in the comment above dget() and
> there's a BUG_ON() in dget() to catch callers that don't already
> have an active reference. An example of a valid unlocked dget():
> d_alloc() does an unlocked dget() to take a reference to the parent
> dentry whichn we already are guaranteed to have a reference to.

Of course you can dget if you already have a reference :)

 
> As to d_validate() - it depends on the caller behaviour as to
> whether the unlocked dget() is valid or not.  From a cursory check
> of the NCP and SMB readdir caches, both appear to hold an active
> reference to the dentry it is passing to d_validate().

I don't see where? Can you point to where the refcount is taken?
AFAIKS it drops the reference 3 lines after it puts the pointer
into cache.


> If that is
> the case then there is nothing wrong with the way d_validate uses
> dget(). Can someone with more SMB/NCP expertise than me validate the
> use of cached dentries?

Then why would it have to use d_validate if it has a reference?
That is supposed to be for an "untrusted" pointer (which is why
it had all the crazy checks that it's in kmem and in the right
slab etc).

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux