[Change the subject and add fsdevel to CC] On Tue, Oct 30, 2018 at 2:27 AM Matthew Bobrowski <mbobrowski@xxxxxxxxxxxxxx> wrote: > [...] > Then I concluded that it doesn't have to be event mask specific i.e. > FAN_OPEN_EXEC. Irrespective of what the event_mask value actually is, if > it contains a flag that has also been set within marks_ignored_mask, then > the event should *not* be sent through to the user: > > if (event_mask & marks_ignored_mask) > return 0; > > I think what Amir has proposed here is also correct and the cleanest > implementation to achieve what we want. > My proposal had a bug w.r.t handling of FS_EVENT_ON_CHILD in ignore mask. I fixed it and pushed to branch: https://github.com/amir73il/linux/commits/fanotify_ignore In the first commit ("fanotify: fix handling of FS_EVENT_ON_CHILD sub-directory events") I have made a claim about an existing bug - that claim still needs to be proven by a test case. it could be there is no bug and this is just an optimization. Man page should be revised to clarify the currently expected behavior: FAN_EVENT_ON_CHILD ... The flag has no effect when marking mounts + or filesystems and has no effect when set in ignore mask Please include that change in your man page draft for new ignore mask interpretations. > > > > Nothing will need to be change for FS_OPEN_EXEC with this change > > applied to implement semantics #2 above. > > I only hope this change is correct... (it did pass existing and new ltp tests) > > > > I also tested this on my side the using existing and newly proposed LTP > test cases, all tests had passed. This change also covers the last test > that I defined in fanotify12 where the mark_mask = FAN_OPEN_EXEC and > ignore_mask = FAN_OPEN. Prior to this change, an event for FAN_OPEN_EXEC > would still slip through despite being a subtype of an open, which > shouldn't be the case. With this change implemented, NO events are sent > through, which is exactly what is expected. > > > > Pros: FAN_OPEN_NOEXEC easy to do using ignore masks. > > > Cons: Semantics is IMO somewhat difficult to explain in the manpage. > > > Probably we'd need to really explain that FAN_OPEN is really a > > > compound of FS_OPEN_NOEXEC and FS_OPEN_EXEC. > > > > > > So overall I think the semantics from 2) is more useful. But I'd start with > > > manpage properly explaining the semantics and interactions between FAN_OPEN > > > and FAN_OPEN_EXEC. If that can be written so that user's head does not > > > spin, I think the implementation can be done in a reasonably simple way as > > > well. And I'm really sorry for leading you in circles Matthew... > > > > > > > Agree to the "man page criteria" > > This is fine. I can document the FAN_OPEN and FAN_OPEN_EXEC semantics > really well, whenever required. But, I'd like to agree upon the solution > and have that finalised before I shift my focus on documentation. > The idea of "document first" is that if you cannot come up with simple man page we are not going to implement those semantics, because they will be unusable. So I agree with Jan. Please take a swing at man page change. It can be a very rough draft, you can send a link to github, so long as we can see that a simple man page is "doable". Thanks, Amir.
