diff mbox series

fanotify.7: Document extended response to permission events

Message ID 20250330153326.1412509-1-amir73il@gmail.com (mailing list archive)
State New
Headers show
Series fanotify.7: Document extended response to permission events | expand

Commit Message

Amir Goldstein March 30, 2025, 3:33 p.m. UTC 
Document FAN_DENY_ERRNO(), that was added in v6.13 and the
FAN_RESPONSE_INFO_AUDIT_RULE extended response info record
that was added in v6.3.

Cc: Richard Guy Briggs <rgb@redhat.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

Alejandro,

I was working on man page updates to fanotify features that landed
in v6.14 and found a few bits from v6.3 that were out of date, so
I added them along with this change.

If you want me to split them out I can, but I did not see much point.

This change to the documentation of fanotify permission event response
is independent of the previous patches I posted to document the new
FAN_PRE_ACCESS event (also v6.14) and the fanotify_init(2) flag
FAN_REPORT_FD_ERROR (v6.13).

There is another fanotify feature in v6.14 (mount events).
I will try to catch up on documenting that one as well.

Thanks,
Amir.

 man/man7/fanotify.7 | 60 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 59 insertions(+), 1 deletion(-)

Comments

Alejandro Colomar March 30, 2025, 5:52 p.m. UTC | #1 
Hi Amir,

On Sun, Mar 30, 2025 at 05:33:26PM +0200, Amir Goldstein wrote:
> Document FAN_DENY_ERRNO(), that was added in v6.13 and the
> FAN_RESPONSE_INFO_AUDIT_RULE extended response info record
> that was added in v6.3.
> 
> Cc: Richard Guy Briggs <rgb@redhat.com>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
> 
> Alejandro,
> 
> I was working on man page updates to fanotify features that landed
> in v6.14 and found a few bits from v6.3 that were out of date, so
> I added them along with this change.
> 
> If you want me to split them out I can, but I did not see much point.

I prefer them in two patches.  You can send them in the same patch set,
though.

> This change to the documentation of fanotify permission event response
> is independent of the previous patches I posted to document the new
> FAN_PRE_ACCESS event (also v6.14) and the fanotify_init(2) flag
> FAN_REPORT_FD_ERROR (v6.13).
> 
> There is another fanotify feature in v6.14 (mount events).
> I will try to catch up on documenting that one as well.
> 
> Thanks,
> Amir.
> 
>  man/man7/fanotify.7 | 60 ++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/man/man7/fanotify.7 b/man/man7/fanotify.7
> index 6f3a9496e..c7b53901a 100644
> --- a/man/man7/fanotify.7
> +++ b/man/man7/fanotify.7
> @@ -820,7 +820,7 @@ This is the file descriptor from the structure
>  .TP
>  .I response
>  This field indicates whether or not the permission is to be granted.
> -Its value must be either
> +Its value must contain either the flag

This seems unrelated.  Please keep it out of the patches.  If you want
to do it, please have a third trivial patch with "wfix" in the subject.

>  .B FAN_ALLOW
>  to allow the file operation or
>  .B FAN_DENY
> @@ -829,6 +829,24 @@ to deny the file operation.
>  If access is denied, the requesting application call will receive an
>  .B EPERM
>  error.
> +Since Linux 6.14,
> +.\" commit b4b2ff4f61ded819bfa22e50fdec7693f51cbbee
> +if a notification group is initialized with class
> +.BR FAN_CLASS_PRE_CONTENT ,
> +the following error values could be returned to the application
> +by setting the
> +.I response
> +value using the
> +.BR FAN_DENY_ERRNO(err)

This formatting is incorrect.  BR means alternating Bold and Roman, but
this only has one token.

> +macro:
> +.BR EPERM ,
> +.BR EIO ,
> +.BR EBUSY ,
> +.BR ETXTBSY ,
> +.BR EAGAIN ,
> +.BR ENOSPC ,
> +.BR EDQUOT .

Should we have a manual page for FAN_DENY_ERRNO()?  (I think we should.)
I don't understand how it's supposed to work from this paragraph.

> +.P
>  Additionally, if the notification group has been created with the
>  .B FAN_ENABLE_AUDIT
>  flag, then the
> @@ -838,6 +856,46 @@ flag can be set in the
>  field.
>  In that case, the audit subsystem will log information about the access
>  decision to the audit logs.

Do we want to start a new paragraph maybe?

> +Since Linux 6.3,
> +.\" commit 70529a199574c15a40f46b14256633b02ba10ca2
> +the
> +.B FAN_INFO
> +flag can be set in the
> +.I response
> +to indicate that extra variable length response record follows struct

s/variable length/variable-length/

And we usually say 'XXX structure' instead of 'struct XXX'.

> +.IR fanotify_response .

The above sentence is too long.  I'd split it into two:

Since Linux 6.3, the FAN_INFO flag can be set in the response field.  It
indicates that an extra variable-length response record follows the
fanotify_response structure.

> +Extra response records start with a common header:
> +.P
> +.in +4n
> +.EX
> +struct fanotify_response_info_header {
> +    __u8 type;
> +    __u8 pad;
> +    __u16 len;
> +};
> +.EE
> +.in
> +.P
> +The value of
> +.I type

I'd say '.type' instead of 'type'.  I know there's no consistency about
it, but I'm going to globally fix that eventually.  Let's do it good for
new documentation.  The '.' allows one to easily know that we're talking
about a struct or union member.


Have a lovely day!
Alex

> +determines the format of the extra response record.
> +In case the value of
> +.I type
> +is
> +.BR FAN_RESPONSE_INFO_AUDIT_RULE ,
> +the following response record is expected
> +with extra details for the audit log:
> +.P
> +.in +4n
> +.EX
> +struct fanotify_response_info_audit_rule {
> +    struct fanotify_response_info_header hdr;
> +    __u32 rule_number;
> +    __u32 subj_trust;
> +    __u32 obj_trust;
> +};
> +.EE
> +.in
>  .\"
>  .SS Monitoring filesystems for errors
>  A single
> -- 
> 2.34.1
>
Amir Goldstein March 30, 2025, 7:53 p.m. UTC | #2 
On Sun, Mar 30, 2025 at 7:52 PM Alejandro Colomar <alx@kernel.org> wrote:
>
> Hi Amir,
>
> On Sun, Mar 30, 2025 at 05:33:26PM +0200, Amir Goldstein wrote:
> > Document FAN_DENY_ERRNO(), that was added in v6.13 and the
> > FAN_RESPONSE_INFO_AUDIT_RULE extended response info record
> > that was added in v6.3.
> >
> > Cc: Richard Guy Briggs <rgb@redhat.com>
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >
> > Alejandro,
> >
> > I was working on man page updates to fanotify features that landed
> > in v6.14 and found a few bits from v6.3 that were out of date, so
> > I added them along with this change.
> >
> > If you want me to split them out I can, but I did not see much point.
>
> I prefer them in two patches.  You can send them in the same patch set,
> though.

ok

I pushed the two patches to
https://github.com/amir73il/man-pages/commits/fan_deny_errno

Let me know if you want me to re-post them

>
> > This change to the documentation of fanotify permission event response
> > is independent of the previous patches I posted to document the new
> > FAN_PRE_ACCESS event (also v6.14) and the fanotify_init(2) flag
> > FAN_REPORT_FD_ERROR (v6.13).
> >
> > There is another fanotify feature in v6.14 (mount events).
> > I will try to catch up on documenting that one as well.
> >
> > Thanks,
> > Amir.
> >
> >  man/man7/fanotify.7 | 60 ++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 59 insertions(+), 1 deletion(-)
> >
> > diff --git a/man/man7/fanotify.7 b/man/man7/fanotify.7
> > index 6f3a9496e..c7b53901a 100644
> > --- a/man/man7/fanotify.7
> > +++ b/man/man7/fanotify.7
> > @@ -820,7 +820,7 @@ This is the file descriptor from the structure
> >  .TP
> >  .I response
> >  This field indicates whether or not the permission is to be granted.
> > -Its value must be either
> > +Its value must contain either the flag
>
> This seems unrelated.  Please keep it out of the patches.  If you want
> to do it, please have a third trivial patch with "wfix" in the subject.

what does wfix stand for?

this is not a typo fix, this is a semantic fix.

It is not true that the value of response is either FAN_ALLOW or FAN_DENY
those are flags in a bitset and the correct statement is that exactly
one of them needs to be set.

>
> >  .B FAN_ALLOW
> >  to allow the file operation or
> >  .B FAN_DENY
> > @@ -829,6 +829,24 @@ to deny the file operation.
> >  If access is denied, the requesting application call will receive an
> >  .B EPERM
> >  error.
> > +Since Linux 6.14,
> > +.\" commit b4b2ff4f61ded819bfa22e50fdec7693f51cbbee
> > +if a notification group is initialized with class
> > +.BR FAN_CLASS_PRE_CONTENT ,
> > +the following error values could be returned to the application
> > +by setting the
> > +.I response
> > +value using the
> > +.BR FAN_DENY_ERRNO(err)
>
> This formatting is incorrect.  BR means alternating Bold and Roman, but
> this only has one token.
>

ok I added a space

> > +macro:
> > +.BR EPERM ,
> > +.BR EIO ,
> > +.BR EBUSY ,
> > +.BR ETXTBSY ,
> > +.BR EAGAIN ,
> > +.BR ENOSPC ,
> > +.BR EDQUOT .
>
> Should we have a manual page for FAN_DENY_ERRNO()?  (I think we should.)
> I don't understand how it's supposed to work from this paragraph.
>


#define FAN_DENY_ERRNO(err) (FAN_DENY | (((err) & 0xff) << 24))

combined FAN_DENY with a specific error, but I see no
reason to expose the internals of this macro

This does not deserve a man page of its own IMO.

If you have a suggested for better formatting, please suggest it


> > +.P
> >  Additionally, if the notification group has been created with the
> >  .B FAN_ENABLE_AUDIT
> >  flag, then the
> > @@ -838,6 +856,46 @@ flag can be set in the
> >  field.
> >  In that case, the audit subsystem will log information about the access
> >  decision to the audit logs.
>
> Do we want to start a new paragraph maybe?
>

ok

> > +Since Linux 6.3,
> > +.\" commit 70529a199574c15a40f46b14256633b02ba10ca2
> > +the
> > +.B FAN_INFO
> > +flag can be set in the
> > +.I response
> > +to indicate that extra variable length response record follows struct
>
> s/variable length/variable-length/
>
> And we usually say 'XXX structure' instead of 'struct XXX'.
>

ok

> > +.IR fanotify_response .
>
> The above sentence is too long.  I'd split it into two:
>
> Since Linux 6.3, the FAN_INFO flag can be set in the response field.  It
> indicates that an extra variable-length response record follows the
> fanotify_response structure.
>

ok

> > +Extra response records start with a common header:
> > +.P
> > +.in +4n
> > +.EX
> > +struct fanotify_response_info_header {
> > +    __u8 type;
> > +    __u8 pad;
> > +    __u16 len;
> > +};
> > +.EE
> > +.in
> > +.P
> > +The value of
> > +.I type
>
> I'd say '.type' instead of 'type'.  I know there's no consistency about
> it, but I'm going to globally fix that eventually.  Let's do it good for
> new documentation.  The '.' allows one to easily know that we're talking
> about a struct or union member.
>

Sounds like a good change to me.

pushed requested fixes to github.

Thanks,
Amir.
Alejandro Colomar March 30, 2025, 8:07 p.m. UTC | #3 
Hi Amir,

On Sun, Mar 30, 2025 at 09:53:36PM +0200, Amir Goldstein wrote:
> > I prefer them in two patches.  You can send them in the same patch set,
> > though.
> 
> ok
> 
> I pushed the two patches to
> https://github.com/amir73il/man-pages/commits/fan_deny_errno
> 
> Let me know if you want me to re-post them

Yes, please.

> > > This change to the documentation of fanotify permission event response
> > > is independent of the previous patches I posted to document the new
> > > FAN_PRE_ACCESS event (also v6.14) and the fanotify_init(2) flag
> > > FAN_REPORT_FD_ERROR (v6.13).
> > >
> > > There is another fanotify feature in v6.14 (mount events).
> > > I will try to catch up on documenting that one as well.
> > >
> > > Thanks,
> > > Amir.
> > >
> > >  man/man7/fanotify.7 | 60 ++++++++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 59 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/man/man7/fanotify.7 b/man/man7/fanotify.7
> > > index 6f3a9496e..c7b53901a 100644
> > > --- a/man/man7/fanotify.7
> > > +++ b/man/man7/fanotify.7
> > > @@ -820,7 +820,7 @@ This is the file descriptor from the structure
> > >  .TP
> > >  .I response
> > >  This field indicates whether or not the permission is to be granted.
> > > -Its value must be either
> > > +Its value must contain either the flag
> >
> > This seems unrelated.  Please keep it out of the patches.  If you want
> > to do it, please have a third trivial patch with "wfix" in the subject.
> 
> what does wfix stand for?

wording fix

$ cat CONTRIBUTING.d/patches/subject | sed -n '/Trivial subject/,+12p';
    Trivial subject
	For trivial patches, you can use subject tags:

		ffix	Formatting fix.
		tfix	Typo fix.
		wfix	Minor wording fix.
		srcfix	Change to manual page source that doesn't affect
			the output.

	Example:

		[PATCH v1] tcp.7: tfix

> this is not a typo fix, this is a semantic fix.
> 
> It is not true that the value of response is either FAN_ALLOW or FAN_DENY
> those are flags in a bitset and the correct statement is that exactly
> one of them needs to be set.

I understand now.  Then, I think it's more important to have this in a
separate patch, to make sure we document the fix in a commit message.

> > > +macro:
> > > +.BR EPERM ,
> > > +.BR EIO ,
> > > +.BR EBUSY ,
> > > +.BR ETXTBSY ,
> > > +.BR EAGAIN ,
> > > +.BR ENOSPC ,
> > > +.BR EDQUOT .
> >
> > Should we have a manual page for FAN_DENY_ERRNO()?  (I think we should.)
> > I don't understand how it's supposed to work from this paragraph.
> >
> 
> 
> #define FAN_DENY_ERRNO(err) (FAN_DENY | (((err) & 0xff) << 24))
> 
> combined FAN_DENY with a specific error, but I see no
> reason to expose the internals of this macro

Ahhh, thanks.

> 
> This does not deserve a man page of its own IMO.
> 
> If you have a suggested for better formatting, please suggest it

How about an example of using it?  I think that'd be more useful than a
lot of text.

> > > +Extra response records start with a common header:
> > > +.P
> > > +.in +4n
> > > +.EX
> > > +struct fanotify_response_info_header {
> > > +    __u8 type;
> > > +    __u8 pad;
> > > +    __u16 len;
> > > +};
> > > +.EE
> > > +.in
> > > +.P
> > > +The value of
> > > +.I type
> >
> > I'd say '.type' instead of 'type'.  I know there's no consistency about
> > it, but I'm going to globally fix that eventually.  Let's do it good for
> > new documentation.  The '.' allows one to easily know that we're talking
> > about a struct or union member.
> >
> 
> Sounds like a good change to me.
> 
> pushed requested fixes to github.


Have a lovely night!
Alex
diff mbox series

Patch

diff --git a/man/man7/fanotify.7 b/man/man7/fanotify.7
index 6f3a9496e..c7b53901a 100644
--- a/man/man7/fanotify.7
+++ b/man/man7/fanotify.7
@@ -820,7 +820,7 @@  This is the file descriptor from the structure
 .TP
 .I response
 This field indicates whether or not the permission is to be granted.
-Its value must be either
+Its value must contain either the flag
 .B FAN_ALLOW
 to allow the file operation or
 .B FAN_DENY
@@ -829,6 +829,24 @@  to deny the file operation.
 If access is denied, the requesting application call will receive an
 .B EPERM
 error.
+Since Linux 6.14,
+.\" commit b4b2ff4f61ded819bfa22e50fdec7693f51cbbee
+if a notification group is initialized with class
+.BR FAN_CLASS_PRE_CONTENT ,
+the following error values could be returned to the application
+by setting the
+.I response
+value using the
+.BR FAN_DENY_ERRNO(err)
+macro:
+.BR EPERM ,
+.BR EIO ,
+.BR EBUSY ,
+.BR ETXTBSY ,
+.BR EAGAIN ,
+.BR ENOSPC ,
+.BR EDQUOT .
+.P
 Additionally, if the notification group has been created with the
 .B FAN_ENABLE_AUDIT
 flag, then the
@@ -838,6 +856,46 @@  flag can be set in the
 field.
 In that case, the audit subsystem will log information about the access
 decision to the audit logs.
+Since Linux 6.3,
+.\" commit 70529a199574c15a40f46b14256633b02ba10ca2
+the
+.B FAN_INFO
+flag can be set in the
+.I response
+to indicate that extra variable length response record follows struct
+.IR fanotify_response .
+Extra response records start with a common header:
+.P
+.in +4n
+.EX
+struct fanotify_response_info_header {
+    __u8 type;
+    __u8 pad;
+    __u16 len;
+};
+.EE
+.in
+.P
+The value of
+.I type
+determines the format of the extra response record.
+In case the value of
+.I type
+is
+.BR FAN_RESPONSE_INFO_AUDIT_RULE ,
+the following response record is expected
+with extra details for the audit log:
+.P
+.in +4n
+.EX
+struct fanotify_response_info_audit_rule {
+    struct fanotify_response_info_header hdr;
+    __u32 rule_number;
+    __u32 subj_trust;
+    __u32 obj_trust;
+};
+.EE
+.in
 .\"
 .SS Monitoring filesystems for errors
 A single