On Tue 08-07-25 14:58:31, Amir Goldstein wrote: > On Tue, Jul 8, 2025 at 1:40 PM Jan Kara <jack@xxxxxxx> wrote: > > > --- a/fs/notify/fanotify/fanotify_user.c > > > +++ b/fs/notify/fanotify/fanotify_user.c > > > @@ -1583,6 +1583,16 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, > > > flags, unsigned int, event_f_flags) > > > (class | fid_mode) != FAN_CLASS_PRE_CONTENT_FID) > > > return -EINVAL; > > > > > > + /* > > > + * With group that reports fid info and allows only pre-content events, > > > + * user may request to get a response id instead of event->fd. > > > + * FAN_REPORT_FD_ERROR does not make sense in this case. > > > + */ > > > + if ((flags & FAN_REPORT_RESPONSE_ID) && > > > + ((flag & FAN_REPORT_FD_ERROR) || > > > + !fid_mode || class != FAN_CLASS_PRE_CONTENT_FID)) > > > + return -EINVAL; > > > + > > > > > > > > > This new group mode is safe, because: > > > 1. event->fd is redundant to target fid > > > 2. other group priorities allow mixing async events in the same group > > > async event can have negative event->fd which signifies an error > > > to open event->fd > > > > I'm not sure I follow here. I'd expect: > > > > if ((flags & FAN_REPORT_RESPONSE_ID) && !fid_mode) > > return -EINVAL; > > > > I.e., if you ask for event ids, we don't return fds at all so you better > > had FID enabled to see where the event happened. And then there's no need > > for FAN_CLASS_PRE_CONTENT_FID at all. Yes, you cannot mix async fanotify > > events with fd with permission events using event id but is that a sane > > combination? What case do you have in mind that justifies this > > complication? > > Not sure what complication you are referring to. > Maybe this would have been more clear: > > + if ((flags & FAN_REPORT_RESPONSE_ID) && (!fid_mode || > + ((flag & FAN_REPORT_FD_ERROR) || class == FAN_CLASS_NOTIFY)) > + return -EINVAL; > + Right, I can understand this easily :) Thanks for clarification. > Yes, !fid_mode is the more important check. > The checks in the second line are because the combination of > FAN_REPORT_RESPONSE_ID with those other flags does not make sense. But FAN_REPORT_FD_ERROR influences also the behavior of pidfd (whether we report full errno there or only FAN_NOPIDFD / FAN_EPIDFD). Hence I think the exclusion with FAN_REPORT_FD_ERROR is wrong. I agree with FAN_CLASS_NOTIFY bit. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
