On Wed, Oct 27, 2021 at 01:14:49PM +0300, Amir Goldstein wrote:
> On Wed, Oct 27, 2021 at 12:28 PM Matthew Bobrowski <repnop@xxxxxxxxxx> wrote:
> >
> > This provides an explanation on the kind of additional information that can
> > be returned alongside the generic struct fanotify_event_metadata and in
> > what form this additional contextual information is delievered to a
> > listening application.
> >
> > Signed-off-by: Matthew Bobrowski <repnop@xxxxxxxxxx>
> > ---
> > man2/fanotify_init.2 | 54 +++++++++++++++++++++++++
> > man7/fanotify.7 | 95 +++++++++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 147 insertions(+), 2 deletions(-)
> >
> > diff --git a/man2/fanotify_init.2 b/man2/fanotify_init.2
> > index 0d83e817f..f65b4fa10 100644
> > --- a/man2/fanotify_init.2
> > +++ b/man2/fanotify_init.2
> > @@ -298,6 +298,60 @@ for additional details.
> > This is a synonym for
> > .RB ( FAN_REPORT_DIR_FID | FAN_REPORT_NAME ).
> > .PP
> > +.TP
> > +.B FAN_REPORT_PIDFD " (since Linux 5.15)"
> > +Events for fanotify groups initialized with this flag will contain an
> > +additional information record object alongside the generic fanotify
> > +event metadata structure.
> > +This additional information record will be of type
> > +.BR FAN_EVENT_INFO_TYPE_PIDFD
> > +and will contain a pidfd for the process that was responsible for
> > +generating an event.
> > +The returned pidfd within the information object is indistinguishable
> > +from a pidfd that is obtained via
> > +.BR pidfd_open(2).
> > +Usage of this additional information record object is for applications
> > +that are interested in reliably determining whether the process
> > +responsible for generating the event has either been recycled or
> > +terminated.
> > +In the instance that either
> > +.BR FAN_REPORT_FID
> > +or
> > +.BR FAN_REPORT_DFID_NAME
> > +are supplied along with
> > +.BR FAN_REPORT_PIDFD
> > +information record objects of type
> > +.BR FAN_EVENT_INFO_TYPE_FID,
> > +.BR FAN_EVENT_INFO_TYPE_DFID
> > +and
> > +.BR FAN_EVENT_INFO_TYPE_DFID_NAME
> > +will likely precede the information object of type
>
> Please get rid of "likely to precede"
ACK.
> > +.BR FAN_EVENT_INFO_TYPE_PIDFD
> > +for a single event within the read buffer.
> > +However, an event listener should never work with this information object
> > +ordering assumption and is encouraged to always check the information type
>
> "However" "encouraged" are too weak to my taste.
>
> An event listener should not assume any specific order for information records
> within an event and must always check the information type...
ACK.
> > +set within the
> > +.IR fanotify_event_info_header
> > +of each information object.
> > +The use of the
> > +.BR FAN_REPORT_TID
> > +flag along with
> > +.BR FAN_REPORT_PIDFD
> > +is currently not supported and attempting to do so will result in the
> > +error
> > +.BR EINVAL
> > +being returned.
> > +This limitation is imposed by the pidfd API as it currently only
> > +supports the creation of pidfds for thread-group leaders.
> > +Creating pidfds for non-thread-group leaders may be supported at some
> > +point in the future, so this restriction may eventually be lifted.
> > +Additional pidfd specific error cases can be reported back to the
> > +listening application in an information record object of type
> > +.BR FAN_EVENT_INFO_TYPE_PIDFD.
> > +See
> > +.BR fanotify(7)
> > +for additional details.
> > +.PP
> > The
> > .I event_f_flags
> > argument
> > diff --git a/man7/fanotify.7 b/man7/fanotify.7
> > index 455e3ed17..09fa4defb 100644
> > --- a/man7/fanotify.7
> > +++ b/man7/fanotify.7
> > @@ -141,12 +141,24 @@ until either a file event occurs or the call is interrupted by a signal
> > The use of one of the flags
> > .BR FAN_REPORT_FID ,
> > .BR FAN_REPORT_DIR_FID
> > +or
> > +.BR FAN_REPORT_PIDFD
> > in
> > .BR fanotify_init (2)
> > influences what data structures are returned to the event listener for each
> > event.
> > -Events reported to a group initialized with one of these flags will
> > -use file handles to identify filesystem objects instead of file descriptors.
> > +Events reported to a group initialized with one of either
> > +.BR FAN_REPORT_FID
> > +or
> > +.BR FAN_REPORT_DIR_FID
> > +flags will use file handles to identify filesystem objects instead of
> > +file descriptors.
> > +Events reported to a group intialized with
> > +.BR FAN_REPORT_PIDFD
> > +will attempt to also create a process file descriptor, commonly
> > +referred to as a pidfd, for the process responsible for generating the
> > +event and provide that alongside the generic fanotify metadata event
> > +structure.
> > .PP
> > After a successful
> > .BR read (2),
> > @@ -188,6 +200,27 @@ struct fanotify_event_info_fid {
> > .EE
> > .in
> > .PP
> > +In the instance that the fanotify group has been initialized with
> > +.BR FAN_REPORT_PIDFD ,
> > +the listening application should expect to receive a single
> > +information record object as detailed below alongside the generic
> > +.I fanotify_event_metadata structure:
> > +.PP
> > +.in +4n
> > +.EX
> > +struct fanotify_event_info_header {
> > + __u8 info_type;
> > + __u8 pad;
> > + __u16 len;
> > +};
>
> This structure was just listed a few lines up.
> There is no need to re-list it here.
ACK.
> > +
> > +struct fanotify_event_info_pidfd {
> > + struct fanotify_event_info_header hdr;
> > + __s32 pidfd;
> > +};
> > +.EE
> > +.in
> > +.PP
> > For performance reasons, it is recommended to use a large
> > buffer size (for example, 4096 bytes),
> > so that multiple events can be retrieved by a single
> > @@ -510,6 +543,64 @@ and the file handle is followed by a null terminated string that identifies the
> > name of a directory entry in that directory, or '.' to identify the directory
> > object itself.
> > .PP
> > +The fields of the
> > +.I fanotify_event_info_pidfd
> > +structure are as follows:
> > +.TP
> > +.I hdr
> > +This is a structure of type
> > +.IR fanotify_event_info_header .
> > +Exactly like the one that is provided in a
> > +.I fanotify_event_info_fid
> > +structure, it is a generic header that contains information used to
> > +describe an additional information record object that is attached to
> > +an event.
> > +In the instance that
> > +.BR FAN_REPORT_PIDFD
> > +is supplied during fanotify group initialization, a single information
> > +record object is expected to be attached alongside the generic
> > +metadata event with its
> > +.I info_type
> > +field set to the value of
> > +.BR FAN_EVENT_INFO_TYPE_PIDFD .
> > +The
> > +.I fanotify_event_info_header
> > +structure also contains a
> > +.I len
> > +field.
> > +The value of the
> > +.I len
> > +field is the total size of the
> > +.I fanotify_event_info_pidfd
> > +structure, which also includes
> > +.IR fanotify_event_info_header .
>
> It would be a shame if we needed to repeat the same text for every new info_type
> that we add. There should be no duplicate documentation of the
> fanotify_event_info_header fields. Perhaps we need to describe those fields
> before documenting fanotify_event_info_fid fields instead of inline in the
> documentation of hdr field.
Right, I see where you're coming from and I do generally agree. If we
continue repeating the same pattern for each bonus event that is based
on fanotify_event_info_header, then we'll end up unnecessarily
polluting the documentation.
Would you like me to try shuffle things around in a patch that
precedes this one?
/M
