Re: [PATCH v2] landlock.7, landlock_*.2: Document LANDLOCK_ADD_RULE_QUIET

[Alejandro Colomar <alx@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4926 bytes --]

Hi Günther, Tingmao!

> Date: 2026-08-29 19:24:45+0200
> From: Günther Noack <gnoack3000@gmail.com>
>
> Thank you very much, Tingmao!
> 
> Documentation is the same as in the kernel docs, and renders fine.  I
> left a few smaller comments below on individual points.  Apart from
> these this looks good. :)

Thanks!  :)

> On Sat, Aug 29, 2026 at 04:13:03PM +0100, Tingmao Wang wrote:
> > LANDLOCK_ADD_RULE_QUIET is a new feature introduced in Landlock ABI
> > version 10, merged in kernel v7.2 [1].  This patch copies relevant
> > kernel documentation into man-pages.
> > 
> > Link: <https://patch.msgid.link/cover.1781228815.git.m@maowtm.org/> [1]
> > Signed-off-by: Tingmao Wang <m@maowtm.org>
> > ---
> > 
> > Changes in v2:
> > - Fix missing .RE, and fix EINVAL label being incorrectly formatted
> > - Fix missed API bump in the example program (abi = MIN(abi, 10);)

Thanks!  I made some comments on v1 a moment ago (I hadn't found v2 yet;
sorry).  They also apply in v2.

> > 
> > Hi,
> > 
> > For context, I'm the author of the quiet flag feature and this is my
> > first man-pages patch.  @Günther or @Mickaël, can one of you do a quick review?

Thanks a lot for the documentation patch!

> > All text in this patch is copied from the kernel source except this
> > bit:
> >     .TP
> >     .B EINVAL
> >     .I flags
> >     is not 0 or one of the allowed values.
> >
> > (the kernel says "%EINVAL: @flags is not valid", I decided to make it
> > more precise)

Thanks for clarifying the difference!  :)

> > 
> >  man/man2/landlock_add_rule.2       | 48 ++++++++++++++++++++++++++++--
> >  man/man2/landlock_create_ruleset.2 | 48 ++++++++++++++++++++++++++++++
> >  man/man7/landlock.7                | 29 +++++++++++++++++-
> >  3 files changed, 121 insertions(+), 4 deletions(-)
> > 
> > diff --git a/man/man2/landlock_add_rule.2 b/man/man2/landlock_add_rule.2
> > index fe01a98d9..c848c9b4b 100644
> > --- a/man/man2/landlock_add_rule.2
> > +++ b/man/man2/landlock_add_rule.2
[...]
> > +Because logging is only suppressed by a layer if the layer denies
> > +access,
> 
> (I suspect Alejandro will bring it up as well; man pages use "semantic
> line breaks" trying to break lines after logical parts of a sentence,
> e.g.
> 
>   Because logging is only suppressed by a layer
>   if the layer denies access,
> 
> etc.)

Indeed, I've made such comments extensively in v1.  :-)

This is some excellent documentation advise, used already by
Brian W. Kernighan back in the days of early Unix.  I find it sad that
most projects don't use this today.  The kernel internal docs must be
unmaintainable, and this is a strong reason why I'd *never* agree to
port manual pages to .rst format as some people have suggested at some
point.

[...]
> > @@ -70,6 +73,17 @@ .SH DESCRIPTION
> >  in
> >  .BR landlock (7)).
> >  .IP
> > +.I quiet_access_fs
> > +is a bitmask of filesystem actions which should not be logged if
> > +per-object quiet flag is set.
> 
> Maybe add a "the" before "per-object" here?  Unlike in the kernel
> docs, this is a full sentence here, so a more complete sentence is
> probably in order?  (Same for quiet_access_net and quiet_scoped below
> as well.)

+1

[...]
> > +.BR LANDLOCK_ADD_RULE_QUIET ,
> > +even if logging would normally take place per
> > +.BR landlock_restrict_self (2)
> > +flags.
> > +.I quiet_scoped
> > +is similar, except that it does not require marking any objects as quiet
> > +-
> 
> Should maybe be a \[em]?

Yes, this should be an \[em].  However, I'd prefer to use parentheses.
Or if we use \[em], I'd want to use them in pairs, and with the same
spacing as if they were parentheses.  Here's an example of how it should
look like:

	... any objects as quiet --if the ruleset ... any log--.
or
	... any objects as quiet (if the ruleset ... any log).

I'm okay with any of these.  This placement of em dashes is standard in
Spain, FWIW.  I'm aware it's not common in English-speaking countries,
but I prefer Spanish rules for em dashes (and we use these rules already
in the manual pages).

[...]
> >  Users should use the Landlock ABI version rather than the kernel version
> > @@ -622,7 +649,7 @@ .SH EXAMPLES
> >      perror("Unable to use Landlock");
> >      return;  /* Graceful fallback: Do nothing.  */
> >  }
> > -abi = MIN(abi, 9);
> > +abi = MIN(abi, 10);
> 
> Please also add an entry to the array in the example (even though it
> is the same as the entry before, in this case).  Otherwise, the
> example has an out-of-bounds array access when this is 10.
> 
> >  \&
> >  /* Only use the available rights in the ruleset.  */
> >  attr.handled_access_fs &= landlock_fs_access_rights[abi \- 1];
> > -- 
> > 2.55.0
> > 
> 
> Thanks,
> –Günther

Have a lovely night!
Alex

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]