Re: [PATCH v2 5/5] fanotify: add pidfd support to the fanotify API

[Matthew Bobrowski <repnop@google.com>]

On Sat, Jul 10, 2021 at 05:49:57PM +0300, Amir Goldstein wrote:
> On Thu, Jun 10, 2021 at 3:22 AM Matthew Bobrowski <repnop@google.com> wrote:
> >
> > Introduce a new flag FAN_REPORT_PIDFD for fanotify_init(2) which
> > allows userspace applications to control whether a pidfd info record
> > containing a pidfd is to be returned with each event.
> >
> > If FAN_REPORT_PIDFD is enabled for a notification group, an additional
> > struct fanotify_event_info_pidfd object will be supplied alongside the
> > generic struct fanotify_event_metadata within a single event. This
> > functionality is analogous to that of FAN_REPORT_FID in terms of how
> > the event structure is supplied to the userspace application. Usage of
> > FAN_REPORT_PIDFD with FAN_REPORT_FID/FAN_REPORT_DFID_NAME is
> > permitted, and in this case a struct fanotify_event_info_pidfd object
> > will follow any struct fanotify_event_info_fid object.
> >
> > Currently, the usage of FAN_REPORT_TID is not permitted along with
> > FAN_REPORT_PIDFD as the pidfd API only supports the creation of pidfds
> > for thread-group leaders. Additionally, the FAN_REPORT_PIDFD is
> > limited to privileged processes only i.e. listeners that are running
> > with the CAP_SYS_ADMIN capability. Attempting to supply either of
> > these initialisation flags with FAN_REPORT_PIDFD will result with
> > EINVAL being returned to the caller.
> >
> > In the event of a pidfd creation error, there are two types of error
> > values that can be reported back to the listener. There is
> > FAN_NOPIDFD, which will be reported in cases where the process
> > responsible for generating the event has terminated prior to fanotify
> > being able to create pidfd for event->pid via pidfd_create(). The
> > there is FAN_EPIDFD, which will be reported if a more generic pidfd
> > creation error occurred when calling pidfd_create().
> >
> > Signed-off-by: Matthew Bobrowski <repnop@google.com>
> >
> 
> [...]
> 
> > diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
> > index fbf9c5c7dd59..5cb3e2369b96 100644
> > --- a/include/uapi/linux/fanotify.h
> > +++ b/include/uapi/linux/fanotify.h
> > @@ -55,6 +55,7 @@
> >  #define FAN_REPORT_FID         0x00000200      /* Report unique file id */
> >  #define FAN_REPORT_DIR_FID     0x00000400      /* Report unique directory id */
> >  #define FAN_REPORT_NAME                0x00000800      /* Report events with name */
> > +#define FAN_REPORT_PIDFD       0x00001000      /* Report pidfd for event->pid */
> >
> 
> Matthew,
> 
> One very minor comment.
> I have a patch in progress to add FAN_REPORT_CHILD_FID (for reporting fid
> of created inode) and it would be nice if we can reserve the flag space in the
> same block with the rest of the FID flags.
> 
> If its not a problem, maybe we could move FAN_REPORT_PIDFD up to 0x80
> right above FAN_REPORT_TID, which also happen to be related flags.

That's fine by me, no objections. Updated my patch series with the updated
flag value [0].

[0]
https://github.com/matthewbobrowski/linux/commit/02ba3581fee21c34bd986e093d9eb0b9897fa741#diff-c83e05fe10af36146658416e55756f304a099606f4a2b18d2fcb683b277c3c62R54

/M