On Wed, Nov 14, 2018 at 3:57 PM Jan Kara <jack@xxxxxxx> wrote: > > Hi, > > On Mon 05-11-18 15:28:16, Amir Goldstein wrote: > > Add support for new fanotify_init() flag FAN_UNPRIVILEGED. > > User may request an unprivileged event listener using this flag even if > > user is privileged. > > > > An unprivileged event listener does not get an open file descriptor in > > the event nor the process pid of another process. An unprivileged event > > listener and cannot request permission events, cannot set mount/filesystem > > marks and cannot request unlimited queue/marks. > > > > This enables the limited functionality similar to inotify when watching a > > single inode for OPEN/ACCESS/MODIFY/CLOSE events, but at least it does > > not require CAP_ADMIN privileges. > > > > Going forward, this limited functionality will be enriched with more > > event types and the ability to watch multiple objects by indetifying > > the object in the event with its nfs file handle. > > > > Note that even though events do not report the event creator pid, > > fanotify does not merge similar events on the same object that were > > generated by different processes. This in aligned with exiting behavior > > when generating processes are outside of the listener pidns (which > > results in reporting 0 pid to listener). > > > > Cc: <linux-api@xxxxxxxxxxxxxxx> > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> > > OK, this patch is next on my fanotify todo list :) Thanks for the patch! > > > FAN_UNPRIVILEGED has merit on its own and depends on no other patches, > > but it is more of a prelude for more upcoming API changes. > > So I find it hard to see how this is going to be useful. If you don't > return open fd, listener has no way of determining on which file something > has happened. Inotify has returned watch descriptor which could be used for > this purpose but fanotify has no such thing. I guess you might want to use > your FID extensions to identify files (provided that does not leak any > sensitive information) but without that this patch is pretty much useless > AFAICT. > I see your point. FAN_UNPRIVILEGED is indeed most useful along with FAN_REPORT_FID, which I am going to claim, is safe for use with unprivileged listeners. In that case, I should probably post FAN_REPORT_FID patches, then post FAN_UNPRIVILEGED patch and enforce that FAN_UNPRIVILEGED requires FAN_REPORT_FID. > Also another thing I'd like to get clarified: What is FAN_UNPRIVILEGED > going to be useful for? So far it will have only disadvantages compared to > inotify and no advantages. Trying to get to feature parity with inotify is > a nice thing but unless we see how this is ever going to more useful than > inotify, I don't see much point because people will just not have any > incentive to use it. I see your point again. At this point in time, the only FAN_UNPRIVILEGED features I can think of are event->pid, namely was the event generated by self, and the new FAN_OPEN_EXEC. Maybe not enough to migrate away from inotify. But if we lay the foundations of FAN_UNPRIVILEGED, people may come up with other FAN_UNPRIVILEGED features, because one thing we can say for certain is that inotify is not going to gain any new features - right? Anyway, there is no rush for me. If the benefit of FAN_UNPRIVILEGED is questionable, we can leave it to another time. The patch is quite small and will be event smaller after FAN_REPORT_FID (which already implies FAN_NOFD). > > And finally, inotify has these tunable namespaced limits on number of > events, marks, and groups (to limit memory usage and inode pinning). I > think you need to implement something similar for fanotify before you > expose it to arbitrary users. > Good point. More of a reason to leave FAN_UNPRIVILEGED for another time. If you change your mind or find better reasons to expedite FAN_UNPRIVILEGED, please let me know. > > FWIW, this patch shouldn't have any conflicts neither with my > > FAN_EVENT_ON_CHILD fix patch nor with Matthew's FAN_OPEN_EXEC patches. > > I think there's a conflict with FAN_REPORT_TID patch but that should be > easy to resolve. > That is strange. All my work is based off of v4.20-rc1. Thanks, Amir.
