On Wed, Jun 19, 2019 at 3:53 PM Jan Kara <jack@xxxxxxx> wrote:
>
> On Wed 19-06-19 13:34:44, Amir Goldstein wrote:
> > When implementing connector fsid cache, we only initialized the cache
> > when the first mark added to object was added by FAN_REPORT_FID group.
> > We forgot to update conn->fsid when the second mark is added by
> > FAN_REPORT_FID group to an already attached connector without fsid
> > cache.
> >
> > Reported-and-tested-by: syzbot+c277e8e2f46414645508@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Fixes: 77115225acc6 ("fanotify: cache fsid in fsnotify_mark_connector")
> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > ---
> >
> > Jan,
> >
> > This fix has been confirmed by syzbot to fix the issue as well as
> > by my modification to Matthew's LTP test:
> > https://github.com/amir73il/ltp/commits/fanotify_dirent
>
> Thanks for the fix Amir. I somewhat don't like the additional flags field
> (which ends up growing fsnotify_mark_connector by one long) for just that
> one special flag. If nothing else, can't we just store the flag inside
> 'type'? There's plenty of space there...
I didn't think it mattered in the grand scheme of things, but
I did consider:
- unsigned int type; /* Type of object [lock] */
+ unsigned short type; /* Type of object [lock] */
+#define FSNOTIFY_CONN_FLAG_HAS_FSID 0x01
+ unsigned short flags; /* flags [lock] */
I think it makes sense.
Let me know if you want me to resend of if you can fix on commit.
Thanks,
Amir.
