lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Mon, 6 Oct 2014 06:35:23 +0100
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Sasha Levin <sasha.levin@...cle.com>
Cc:	linux-fsdevel <linux-fsdevel@...r.kernel.org>, bfields@...hat.com,
	mszeredi@...e.cz, "Eric W. Biederman" <ebiederm@...ssion.com>,
	hch@....de, Dave Jones <davej@...hat.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: dcache: NULL ptr deref in dentry_kill

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists