Re: WARNING in fanotify_handle_event

[Amir Goldstein]

On Tue, Jun 18, 2019 at 8:07 PM syzbot
<syzbot+c277e8e2f46414645508@xxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:    963172d9 Merge branch 'x86-urgent-for-linus' of git://git...
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=17c090eaa00000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=fa9f7e1b6a8bb586
> dashboard link: https://syzkaller.appspot.com/bug?extid=c277e8e2f46414645508
> compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=15a32f46a00000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=13a7dc9ea00000
>
> The bug was bisected to:
>
> commit 77115225acc67d9ac4b15f04dd138006b9cd1ef2
> Author: Amir Goldstein <amir73il@xxxxxxxxx>
> Date:   Thu Jan 10 17:04:37 2019 +0000
>
>      fanotify: cache fsid in fsnotify_mark_connector
>
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=12bfcb66a00000
> final crash:    https://syzkaller.appspot.com/x/report.txt?x=11bfcb66a00000
> console output: https://syzkaller.appspot.com/x/log.txt?x=16bfcb66a00000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+c277e8e2f46414645508@xxxxxxxxxxxxxxxxxxxxxxxxx
> Fixes: 77115225acc6 ("fanotify: cache fsid in fsnotify_mark_connector")
>
> WARNING: CPU: 0 PID: 8994 at fs/notify/fanotify/fanotify.c:359
> fanotify_get_fsid fs/notify/fanotify/fanotify.c:359 [inline]

Oops, we forgot to update conn->fsid when the first mark added
for inode has no fsid (e.g. inotify) and the second mark has fid,
which is more or less the only thing the repro does.
And if we are going to update conn->fsid, we do no have the
cmpxchg to guaranty setting fsid atomically.

I am thinking a set-once flag on connector FSNOTIFY_CONN_HAS_FSID
checked before smp_rmb() in fanotify_get_fsid().
If the flag is not set then call vfs_get_fsid() instead of using fsid cache.
conn->fsid can be updated in fsnotify_add_mark_list() under conn->lock,
and flag set after smp_wmb().

Does that sound correct?

Thanks,
Amir.