Re: Ignore mask handling in fanotify_group_event_mask()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, May 21, 2020 at 9:10 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> On Thu, May 21, 2020 at 7:24 PM Jan Kara <jack@xxxxxxx> wrote:
> >
> > Hello Amir!
> >
> > I was looking into backporting of commit 55bf882c7f13dd "fanotify: fix
> > merging marks masks with FAN_ONDIR" and realized one oddity in
> > fanotify_group_event_mask(). The thing is: Even if the mark mask is such
> > that current event shouldn't trigger on the mark, we still have to take
> > mark's ignore mask into account.
> >
> > The most realistic example that would demonstrate the issue that comes to my
> > mind is:
> >
> > mount mark watching for FAN_OPEN | FAN_ONDIR.
> > inode mark on a directory with mask == 0 and ignore_mask == FAN_OPEN.
> >
> > I'd expect the group will not get any event for opening the dir but the
> > code in fanotify_group_event_mask() would not prevent event generation. Now
> > as I've tested the event currently actually does not get generated because
> > there is a rough test in send_to_group() that actually finds out that there
> > shouldn't be anything to report and so fanotify handler is actually never
> > called in such case. But I don't think it's good to have an inconsistent
> > test in fanotify_group_event_mask(). What do you think?
> >
>
> I agree this is not perfect.
> I think that moving the marks_ignored_mask line
> To the top of the foreach loop should fix the broken logic.
> It will not make the code any less complicated to follow though.
> Perhaps with a comment along the lines of:
>
>              /* Ignore mask is applied regardless of ISDIR and ON_CHILD flags */
>              marks_ignored_mask |= mark->ignored_mask;
>
> Now is there a real bug here?
> Probably not because send_to_group() always applied an ignore mask
> that is greater or equal to that of fanotify_group_event_mask().
>

That's a wrong statement of course.
We do need to re-apply the ignore mask when narrowing the event mask.

Exposing the bug requires a "compound" event.

The only case of compound event I could think of is this:

mount mark with mask == 0 and ignore_mask == FAN_OPEN. inode mark
on a directory with mask == FAN_EXEC | FAN_EVENT_ON_CHILD.

The event: FAN_OPEN | FAN_EXEC | FAN_EVENT_ON_CHILD
would be reported to group with the FAN_OPEN flag despite the
fact that FAN_OPEN is in ignore mask of mount mark because
the mark iteration loop skips over non-inode marks for events
on child.

I'll try to work that case into the relevant LTP test to prove it and
post a fix.

Thanks,
Amir.



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux