On Fri, Sep 28, 2018 at 4:28 AM Matthew Bobrowski <mbobrowski@xxxxxxxxxxxxxx> wrote: > > Hi Amir, > > On 27/9/18 11:57 pm, Amir Goldstein wrote: > > [cc: linux-api] > > > > On Thu, Sep 27, 2018 at 4:05 PM Matthew Bobrowski > > <mbobrowski@xxxxxxxxxxxxxx> wrote: > >> > >> This is a reduced version of a patch that I originally submitted a while ago. > >> > >> In short, the fanotify API currently does not provide any means for user space > >> programs to receive events specifically when a file has been opened with the > >> intent to be executed. The FAN_EXEC flag will be set within the event mask when > >> a object has been opened with one of the open flags being __FMODE_EXEC. > >> ... > >> diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h > >> index fd1ce10553bf..aad174c81322 100644 > >> --- a/include/linux/fsnotify.h > >> +++ b/include/linux/fsnotify.h > >> @@ -216,6 +216,9 @@ static inline void fsnotify_open(struct file *file) > >> if (S_ISDIR(inode->i_mode)) > >> mask |= FS_ISDIR; > >> > >> + if (file->f_flags & __FMODE_EXEC) > >> + mask |= FS_EXEC; > >> + > > > > I think that may be breaking existing programs that do not expect to see > > this bit in the event mask (i.e. if they only requested to see FAN_OPEN). > > A very good point and is definitely something that did cross my mind while > writing this patch. > My only issue with my own suggestion is that this implicit behavior is harder to document than the explicit behavior (i.e. "user will get FAN_EXEC only if user sets fanotify_init flag FAN_ENABLE_EXEC"), so I haven't decided which I prefer yet - waiting for Jan to weight in on this point. The concept of "bonus flags" (flags that you got but did not ask for) is not new to inotify (I think you can get IN_ATTRIB if you asked for IN_MODIFY), but AFAIKT, it would be new to fanotify. OTOH, getting an event that you asked for in the past and since then removed the event bit from the mark is not a new behavior. Although this is not the same as the implicit global enable flag I proposed. The more I write about it, the more I am leaning towards explicit enable... > > A possible mitigation would be to set a group flag FAN_ENABLE_EXEC > > on the first time that user requests the FAN_EXEC event and then > > compute the FAN_ALL_OUTGOING_EVENTS at runtime based on that > > group flag (see example with FAN_ONDIR in my dev branch [1]). > > Unlike my example, I don't think you have to expose the init flag > > FAN_ENABLE_EXEC to users, because it can be set implicitly on the > > first FAN_EXEC mark request. > > Ah yes, I think this is quite elegant and could actually work well. Let me > review your suggestions and write an alternative patch using this > particular approach. > If Jan accepts my proposal, you can base your patch on: https://github.com/amir73il/linux/commits/fanotify_api-v3 Now I've modified my dev branch to use the implicit enable: - Requesting any dentry event sets internal group flag FAN_ENABLE_DENTRY - "bonus" event bit FAN_ONDIR is exposed if group has FAN_ENABLE_DENTRY https://github.com/amir73il/linux/commits/fanotify_dentry Thanks, Amir.
