On Sun, Mar 30, 2025 at 7:52 PM Alejandro Colomar <alx@xxxxxxxxxx> 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@xxxxxxxxxx> > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> > > --- > > > > 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.