Re: fs: NULL deref in atime_needs_update

On Mon, Feb 29, 2016 at 5:19 PM, Al Viro [off-list ref] wrote:

On Mon, Feb 29, 2016 at 04:54:54PM +0100, Dmitry Vyukov wrote:

quoted

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.

quoted

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