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: <CACT4Y+Y4y5RgpV71+chadmWTK+bhH8QFLHrvuMg4d0MJKd2Kzw@mail.gmail.com>
Date:	Mon, 29 Feb 2016 19:19:52 +0100
From:	Dmitry Vyukov <dvyukov@...gle.com>
To:	Al Viro <viro@...iv.linux.org.uk>
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

On Mon, Feb 29, 2016 at 5:19 PM, Al Viro <viro@...iv.linux.org.uk> wrote:
> On Mon, Feb 29, 2016 at 04:54:54PM +0100, Dmitry Vyukov wrote:
>
>> Regardless of whether reordering is wrong or not, do we see how it can
>> fix the WARNINGs/oopses? Because it does seem to. I've tried to revert
>> just this part:
>>
>> -               *inode = d_backing_inode(dentry);
>>                 negative = d_is_negative(dentry);
>> +               *inode = d_backing_inode(dentry);
>>
>> And got:
>>
>> [  976.609688] WARNING: CPU: 0 PID: 12126 at fs/namei.c:1587
>> lookup_fast+0x3fa/0x450()
>> [  976.626768] WARNING: CPU: 0 PID: 12126 at fs/namei.c:3123
>> path_openat+0x12bc/0x1520()
>>
>> in 15 minutes.
>
> dentry going from negative to positive                  lookup_fast()
>                                                         fetch NULL ->d_inode
> store non-NULL ->d_inode
> store new ->d_flags
>                                                         fetch new ->d_flags
>                                                         check ->d_seq
> bump ->d_seq by 2
>
> Change the order of fetches and you'll get rid of that scenario.
>
>> In particular, applying this on top the previous patch will be
>> inconclusive, because I already don't see the warnings.
>
> Apply it with that reordering reversed, please.


OK, just wanted to make sure that we keep track of the situation.
Restarted testing with combined patch. Here is it just in case:
https://gist.githubusercontent.com/dvyukov/67fe363d5ce2e2b06c71/raw/4d1b6c23f8dff7e0f8e2e3cab7e50208fddb0570/gistfile1.txt