linux-kernel - Re: fs: NULL deref in atime_needs

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160228170133.GM17997@ZenIV.linux.org.uk>
Date:	Sun, 28 Feb 2016 17:01:34 +0000
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Dmitry Vyukov <dvyukov@...gle.com>
Cc:	Ian Kent <raven@...maw.net>,
	Mickaël Salaün <mic@...ikod.net>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	syzkaller <syzkaller@...glegroups.com>,
	Kostya Serebryany <kcc@...gle.com>,
	Alexander Potapenko <glider@...gle.com>,
	Sasha Levin <sasha.levin@...cle.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	David Howells <dhowells@...hat.com>
Subject: Re: fs: NULL deref in atime_needs_update

[dhowells Cc'd]

On Sun, Feb 28, 2016 at 05:04:19PM +0100, Dmitry Vyukov wrote:

> 
> [ 1422.292356] ------------[ cut here ]------------
> [ 1422.292841] WARNING: CPU: 0 PID: 32603 at fs/namei.c:1587
> lookup_fast+0x3fa/0x450()

Huh?  So you have
                dentry = __d_lookup_rcu(parent, &nd->last, &seq);
returning non-NULL dentry, then
                *inode = d_backing_inode(dentry);
                negative = d_is_negative(dentry);
                if (read_seqcount_retry(&dentry->d_seq, seq))
                        return -ECHILD;
followed by by *inode == NULL and negative == true?

Nuts...  OK, that removes vfsmounts from consideration, but...  How the
fuck is that possible?  We have
		smp_rmb();
                seq = &dentry->d_seq->sequence & ~1;
see that ->d_name and ->d_parent match what we are looking for,
then
		*inode = dentry->d_inode;
		type = READ_ONCE(dentry->d_flags);
		smp_rmb();
		negative = (type & DCACHE_ENTRY_TYPE) == DCACHE_MISS_TYPE;
		smp_rmb();
		if (dentry->d_seq->sequence != seq)
			sod off
and observe *inode == NULL && !negative

Erm...  What's to order ->d_inode and ->d_flags fetches there?  David?
Looks like the barrier in d_is_negative() is on the wrong side of fetch.
Confused...