Re: [patch] fs: fix d_validate

[Nick Piggin <npiggin@kernel.dk>]

On Tue, Nov 16, 2010 at 08:20:52AM -0800, Linus Torvalds wrote:
> On Mon, Nov 15, 2010 at 10:23 PM, Nick Piggin <npiggin@kernel.dk> wrote:
> >
> > fs: d_validate fixes
> >
> > d_validate has been broken for a long time.
> >
> > kmem_ptr_validate does not guarantee that a pointer can be dereferenced
> > if it can go away at any time. Even rcu_read_lock doesn't help, because
> > the pointer might be queued in RCU callbacks but not executed yet.
> 
> Hmm. Iirc (and it's possible that I don't) the original reason for the
> whole kmem_ptr_validate() was for nfsd sending dentry pointers back
> and forth as part of the FH.

Yeah that looked to be the case, when I checked old kernels.

 
> So we really _did_ need to do basic validation, and no, we didn't care
> about things like DEBUG_PAGEALLOC, because that's totally
> inappropriate for a nfs server anyway. So it was "broken", but it
> worked.
> 
> Now, I hope that kind of "we really fundamentally cannot even trust
> the pointer" doesn't exist any more, in which case I agree with you.
> But historically, it really did matter at some point, and when the
> code says "insecure source", it really means it - it could
> historically have come from user space or over the network.

Well there is nothing in the tree that uses it after my patch to
fix d_validate (that was the only user). d_validate itself was only
used for a funny dir cache in ncpfs and smbfs which has a cache of
pointers to dentries but no references on them, which is pretty hard
to sanely do anything with.

I imagine that if such a kind of cache turns out to be needed in any
relevant filesystem, it could be properly implemented in the dcache
code and not have to play those games. If any user really comes up
for an untrusted pointer, we can always resurrect this -- the kmem
API is not broken as such, but it's just really easy to use wrongly.