On Mon, Aug 20, 2018 at 1:07 PM Jan Kara <jack@xxxxxxx> wrote:
>
> On Sun 19-08-18 15:35:06, Amir Goldstein wrote:
> > Reported-and-tested-by: syzbot+c34692a51b9a6ca93540@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Fixes: 3ac70bfcde81 ("fsnotify: add helper to get mask from connector")
> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > ---
> >
> > Jan,
> >
> > syzbot reported (in private email) that the reproducer did not trigger
> > the warning, so added tested-by.
>
> Thanks for looking into this Amir! I was thinking about this for a while
> and I'm not sure that __fsnotify_recalc_mask() call from
> fsnotify_put_mark() is the only place calling __fsnotify_recalc_mask() that
> can happen on detached connector. unlink(2) can get called pretty much at
> any time so places like inotify_update_existing_watch() can easily work on
> inode that is getting unlinked and by the time we get to
> fsnotify_recalc_mask(), we can pass detached connector to it AFAICT.
> conn->lock we hold in __fsnotify_recalc_mask() protecs us from
> fsnotify_detach_connector_from_object() so we can reliably check connector
> state in __fsnotify_recalc_mask() and just don't do anything when the
> connector is already detached without issuing a warning. What do you think?
>
Makes sense. WARN_ON() is a new addition by cleanup patches
and the fact that disconnected state is valid in __fsnotify_recalc_mask()
was an oversight.
Feel free to apply the simpler fix with reported-by attribution to syzbot ;-)
Thanks,
Amir.
