Re: [PATCH v3] fanotify: Document FAN_REPORT_PIDFD Feature

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Matthew,

On 4/14/22 01:40, Matthew Bobrowski wrote:
Haha, I created this patch using one of Amir's branches, as he
performed a rebase and handled some conflicts. It must've preserved
the display name "Amir Goldstein" in the "From:" header...

:)


On Wed, Apr 13, 2022 at 08:24:21PM +0200, Alejandro Colomar wrote:
Hi Amir!

On 4/12/22 01:17, Amir Goldstein wrote:
Update the fanotify API documentation to include details on the new
FAN_REPORT_PIDFD feature. This patch also includes a generic section
describing the concept of information records which are supported by
the fanotify API.

Signed-off-by: Matthew Bobrowski <repnop@xxxxxxxxxx>
Reviewed-by: Jan Kara <jack@xxxxxxx>
Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>

Thanks for the patch.  Please see some comments below.

Thanks for the review, I'll update and send through a follow up patch
shortly. I left a question on your comment about the use of semantic
newlines. I wasn't sure whether that comment was a general rule that
is to be applied across this entire patch (in which it definitely can,
I just wasn't aware of the rule prior to you explicitly pointing it
out), or whether there was a specific example you were referring to in
the code block directly above your comment.

General rule to be applied across the entire patch, if you do the favour. I just mentioned it at a point where it is clear that it could be applied, to give some context.

Thank you,

Alex


---
   man2/fanotify_init.2 |  34 +++++++
   man7/fanotify.7      | 208 +++++++++++++++++++++++++++++++++++--------
   2 files changed, 204 insertions(+), 38 deletions(-)

diff --git a/man2/fanotify_init.2 b/man2/fanotify_init.2
index 7f9a21a52..1e4691c88 100644
--- a/man2/fanotify_init.2
+++ b/man2/fanotify_init.2
@@ -283,6 +283,40 @@ for additional details.
   This is a synonym for
   .RB ( FAN_REPORT_DIR_FID | FAN_REPORT_NAME ).
   .PP
+.TP
+.BR FAN_REPORT_PIDFD " (since Linux 5.15)"
+.\" commit af579beb666aefb17e9a335c12c788c92932baf1
+Events for fanotify groups initialized with this flag will contain an
+additional information record alongside the generic
+.I fanotify_event_metadata
+structure.
+This information record will be of type
+.B FAN_EVENT_INFO_TYPE_PIDFD
+and will contain a pidfd for the process that was responsible for
+generating an event.
+A pidfd returned in this information record object is no different
+to the pidfd that is returned when calling
+.BR pidfd_open(2) .

Misplaced space.  Should be:

.BR pidfd_open (2).

+Usage of this information record are for applications that may be
+interested in reliably determining whether the process responsible for
+generating an event has been recycled or terminated.
+The use of the
+.B FAN_REPORT_TID
+flag along with
+.B FAN_REPORT_PIDFD
+is currently not supported and attempting to do so will result in the
+error

s.n.

+.B EINVAL
+being returned.
+This limitation is currently imposed by the pidfd API as it currently
+only supports the creation of pidfds for thread-group
+leaders.

Please use semantic newlines.
See man-pages(7):

    Use semantic newlines
        In the source of a manual page, new sentences  should  be
        started on new lines, long sentences should be split into
        lines at clause breaks (commas, semicolons,  colons,  and
        so on), and long clauses should be split at phrase bound-
        aries.  This convention,  sometimes  known  as  "semantic
        newlines",  makes it easier to see the effect of patches,
        which often operate at the level of individual sentences,
        clauses, or phrases.

Fair point. Based on the above change block, I assume you're referring
to the the word "leaders" starting on a new line? The rest appears to
be inline with what's expected of semantic newlines?

I wrote "s.n." in the more grievous cases where semantic newlines could be improved.

Don't worry too much about it if you can't see some of the lines. Just do some cleanup, and I'll continue from there. Thanks.


+Creating pidfds for non-thread-group leaders may be supported at some
+point in the future, so this restriction may eventually be

s.n.

+lifted.
+For more details on information records, see
+.BR fanotify(7) .

Misplaced space.  Should be:

.BR fanotify (7).

+.PP
   The
   .I event_f_flags
   argument
diff --git a/man7/fanotify.7 b/man7/fanotify.7
index f8345b3f5..57dd2b040 100644
--- a/man7/fanotify.7
+++ b/man7/fanotify.7
@@ -118,16 +118,6 @@ until either a file event occurs or the call is interrupted by a signal
   (see
   .BR signal (7)).
   .PP
-The use of one of the flags
-.BR FAN_REPORT_FID ,
-.B FAN_REPORT_DIR_FID
-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.
-.PP
   After a successful
   .BR read (2),
   the read buffer contains one or more of the following structures:
@@ -146,20 +136,63 @@ struct fanotify_event_metadata {
   .EE
   .in
   .PP
-In case of an fanotify group that identifies filesystem objects by file
-handles, you should also expect to receive one or more additional information
-records of the structure detailed below following the generic
+Information records are supplemental pieces of information that may be
+provided alongside the generic
+.I fanotify_event_metadata
+structure.
+The
+.I flags
+passed to
+.BR fanotify_init (2) > +have influence over the type of information records that may be returned
+for an event.
+For example, if a notification group is initialized with
+.B FAN_REPORT_FID
+or
+.BR FAN_REPORT_DIR_FID ,
+then event listeners should also expect to receive a
+.I fanotify_event_info_fid
+structure alongside the
+.I fanotify_event_metadata
+structure, whereby file handles are used to identify filesystem
+objects rather than file descriptors.

s.n.

+Information records may also be stacked, meaning that using the
+various

s.n.

+.B FAN_REPORT_*
+flags in conjunction with one another is supported.
+In such cases, multiple information records can be returned for an
+event alongside the generic
+.I fanotify_event_metadata
+structure.
+For example, if a notification group is initialized with
+.B FAN_REPORT_FID
+and
+.BR FAN_REPORT_PIDFD ,
+then an event listener should also expect to receive both
+.I fanotify_event_info_fid
+and
+.I fanotify_event_info_pidfd
+structures alongside the generic
+.I fanotify_event_metadata
+structure.
+Importantly, fanotify provides no guarantee around the ordering of
+information records when a notification group is intialized with a
+stacked based configuration.
+Each information record has a nested structure of type
+.IR fanotify_event_info_header .
+It is imperative for event listeners to inspect the
+.I info_type
+field of this structure in order to determine the type of information
+record that had been received for a given event.

s.n.

+.PP
+In cases where an fanotify group identifies filesystem objects by file
+handles, event listeners should also expect to receive one or more of

s.n.

+the below information record objects alongside the generic
   .I fanotify_event_metadata
   structure within the read buffer:
   .PP
   .in +4n
   .EX
-struct fanotify_event_info_header {
-    __u8 info_type;
-    __u8 pad;
-    __u16 len;
-};
-
   struct fanotify_event_info_fid {
       struct fanotify_event_info_header hdr;
       __kernel_fsid_t fsid;
@@ -168,6 +201,40 @@ struct fanotify_event_info_fid {
   .EE
   .in
   .PP
+In cases where an fanotify group is initialized with
+.BR FAN_REPORT_PIDFD ,
+event listeners should expect to receive the below information record
+object alongside the generic

s.n.

+.I fanotify_event_metadata
+structure within the read buffer:
+.PP
+.in +4n
+.EX
+struct fanotify_event_info_pidfd {
+        struct fanotify_event_info_header hdr;
+        __s32 pidfd;
+};
+.EE
+.in
+.PP
+All information records contain a nested structure of type
+.IR fanotify_event_info_header .
+This structure holds meta-information about the information record
+that may have been returned alongside the generic
+.I fanotify_event_metadata
+structure.
+This structure is defined as follows:
+.PP
+.in +4n
+.EX
+struct fanotify_event_info_header {
+	__u8 info_type;
+	__u8 pad;
+	__u16 len;
+};
+.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
@@ -385,6 +452,48 @@ The
   flag is reported in an event mask only if the fanotify group identifies
   filesystem objects by file handles.
   .PP
+Information records that are supplied alongside the generic
+.I fanotify_event_metadata
+structure will always contain a nested structure of type
+.IR fanotify_event_info_header .
+The fields of the
+.I fanotify_event_info_header
+are as follows:
+.TP
+.I info_type
+A unique integer value representing the type of information record
+object received for an event.

s.n.

+The value of this field can be set to one of the following:
+.BR FAN_EVENT_INFO_TYPE_FID ,
+.BR FAN_EVENT_INFO_TYPE_DFID ,
+.B FAN_EVENT_INFO_TYPE_DFID_NAME
+or
+.BR FAN_EVENT_INFO_TYPE_PIDFD .

Use Oxford-style commas.  See:

$ git show 3ded684c1a4b6104e1d4b7400015f8bf76dc75b9 | head -n25
commit 3ded684c1a4b6104e1d4b7400015f8bf76dc75b9
Author: Michael Kerrisk <mtk.manpages@xxxxxxxxx>
Date:   Sat Jan 9 11:14:08 2021 +0100

     Various pages: tfix (Oxford comma)

     Found using:

         git grep -lE '^[^.].*,.*,.*[^,] (and|or)\>'

     Signed-off-by: Michael Kerrisk <mtk.manpages@xxxxxxxxx>

diff --git a/man1/intro.1 b/man1/intro.1
index 57023b652..9bf7190df 100644
--- a/man1/intro.1
+++ b/man1/intro.1
@@ -220,7 +220,7 @@ and
  .I pwd
  commands and explore
  .I cd
-usage: "cd", "cd .", "cd ..", "cd /" and "cd \(ti".
+usage: "cd", "cd .", "cd ..", "cd /", and "cd \(ti".
  .SS Directories
  The command
  .I mkdir


+The value set for this field is dependent on the flags that have been
+supplied to
+.BR fanotify_init (2) .

Spurious space.  Should be:

.BR fanotify_init (2).

+Refer to the field details of each information record object type
+below to understand the different cases in which the
+.I info_type
+values can be set.
+.TP
+.I pad
+This field is currently not used by any information record object type
+and therefore is set to zero.
+.TP
+.I len
+The value of
+.I len
+is set to the size of the information record object, including the
+.IR fanotify_event_info_header .
+The total size of all additional information records is not expected
+to be larger than
+(
+.I event_len
+\-
+.I metadata_len
+).

The above can be simplified to:

.RI ( event_len
\-
.IR metadata_len ).

+.PP
   The fields of the
   .I fanotify_event_info_fid
   structure are as follows:
@@ -392,8 +501,6 @@ structure are as follows:
   .I hdr
   This is a structure of type
   .IR fanotify_event_info_header .
-It is a generic header that contains information used to describe an
-additional information record attached to the event.
   For example, when an fanotify file descriptor is created using
   .BR FAN_REPORT_FID ,
   a single information record is expected to be attached to the event with
@@ -414,23 +521,6 @@ identifying a parent directory object, and one with
   field value of
   .BR FAN_EVENT_INFO_TYPE_FID ,
   identifying a non-directory object.
-The
-.I fanotify_event_info_header
-contains a
-.I len
-field.
-The value of
-.I len
-is the size of the additional information record including the
-.I fanotify_event_info_header
-itself.
-The total size of all additional information records is not expected
-to be bigger than
-(
-.I event_len
-\-
-.I metadata_len
-).
   .TP
   .I fsid
   This is a unique identifier of the filesystem containing the object
@@ -498,6 +588,48 @@ 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 .
+When an fanotify group is initialized using
+.BR FAN_REPORT_PIDFD ,
+the
+.I info_type
+field value of the
+.I fanotify_event_info_header
+is set to
+.BR FAN_EVENT_INFO_TYPE_PIDFD .
+.TP
+.I pidfd
+This is a process file descriptor that refers to the process
+responsible for generating the event.
+The returned process file descriptor is no different from one which
+could be obtained manually if
+.BR pidfd_open(2)

Missing a space before "(2)".

+were to be called on
+.IR fanotify_event_metadata.pid .
+In the instance that an error is encountered during pidfd creation for
+one of two possible error types represented by a negative integer
+value may be returned in this

s.n.

+.I pidfd
+field.
+In cases where the process responsible for generating the event has
+terminated prior to the event listener being able to read events from the
+notification queue,
+.B FAN_NOPIDFD
+is returned.
+The pidfd creation for an event is only performed at the time the
+events are read from the notification queue.
+All other possible pidfd creation failures are represented by
+.BR FAN_EPIDFD .
+Once the event listener has dealt with an event and the pidfd is no
+longer required, the pidfd should be closed via

s.n.

+.BR close(2) .

Space is misplaced.  Should be:

.BR close (2).

+.PP
   The following macros are provided to iterate over a buffer containing
   fanotify event metadata returned by a
   .BR read (2)

/M



[Index of Archives]     [Kernel Documentation]     [Netdev]     [Linux Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux