Re: dcache: NULL ptr deref in dentry_kill

[Al Viro]

On Mon, Oct 06, 2014 at 12:39:16AM -0400, Sasha Levin wrote:
>         if (!IS_ROOT(dentry)) {
>                 parent = dentry->d_parent;
>                 if (unlikely(!spin_trylock(&parent->d_lock))) { <=== here
>                         if (inode)
>                                 spin_unlock(&inode->i_lock);
>                         goto failed;
> 
> We're trying to deref a NULL 'parent'.
 
->d_parent is *never* NULL.  There are very few places where it's modified,
all of them in fs/dcache.c:
fs/dcache.c:1416:       dentry->d_parent = dentry;
fs/dcache.c:1453:       dentry->d_parent = parent;
fs/dcache.c:2478:               dentry->d_parent = target->d_parent;
fs/dcache.c:2479:               target->d_parent = target;
fs/dcache.c:2484:               swap(dentry->d_parent, target->d_parent);

The fifth one exchanges two something->d_parent.  Can't introduce NULL.
Neither can the third one (again, foo->d_parent = bar->d_parent).  The
first and the fourth are also obvious - p->d_parent = p will oops with
p == NULL and store a non-NULL otherwise.  Which leaves the second -
d_alloc().  And there the lines immediately after that assignment are
        list_add(&dentry->d_u.d_child, &parent->d_subdirs);
        spin_unlock(&parent->d_lock);
which would oops with parent == NULL.

Dentries are allocated by __d_alloc().  By the time somebody might
observe them, they already have non-NULL ->d_parent.  And they never
get it set to NULL afterwards.  I don't see any variables (auto or not)
of type struct dentry and I don't see anything that would contain
struct dentry as a field.  It doesn't guarantee that nobody manages
to allocate one somehow or hide a conversion of some strange pointer to
struct dentry *, but any such place would be very likely to trigger tons
of oopsen - it would have to manage to hide initialization of ->d_subdirs,
->d_lru, ->d_alias, etc. and it's hard to do accidentally.

Another possibility is that this pointer either never went to struct dentry
or used to point to one in times long past, and memory had been zeroed
since then.  Or that something has shat some zeroes into a real struct
dentry, corrupting ->d_parent in process.

But any struct dentry with NULL ->d_parent is a serious bug.  That really
should never, ever happen.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/