Re: [PATCH 5/5] landlock.7: Document Landlock ABI version 5 (IOCTL)

["Günther Noack" <gnoack@google.com>]

Thank you both for the reviews!
I sent out a second version.

Konstantin: Please also do have a look if you are around, since this also covers
the documentation for your networking connect/bind feature.  Thank you!


On Tue, Jul 16, 2024 at 04:39:17PM +0200, Mickaël Salaün wrote:
> On Mon, Jul 15, 2024 at 06:20:53PM +0200, Alejandro Colomar wrote:
> > On Mon, Jul 15, 2024 at 03:55:54PM GMT, Günther Noack wrote:
> > > ---
> > >  man/man7/landlock.7 | 51 +++++++++++++++++++++++++++++++++++++++++++--
> > >  1 file changed, 49 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/man/man7/landlock.7 b/man/man7/landlock.7
> > > index d452b93b2..044f57208 100644
> > > --- a/man/man7/landlock.7
> > > +++ b/man/man7/landlock.7
> > > @@ -92,6 +92,8 @@ This access right is available since the third version of the Landlock ABI.
> > >  .P
> > >  Whether an opened file can be truncated with
> > >  .BR ftruncate (2)
> > > +or used with
> > > +.BR ioctl (2)
> > >  is determined during
> > >  .BR open (2),
> > >  in the same way as read and write permissions are checked during
> > > @@ -188,6 +190,48 @@ If multiple requirements are not met, the
> > >  .B EACCES
> > >  error code takes precedence over
> > >  .BR EXDEV .
> > > +.P
> > > +The following access right
> > > +applies to both files and directories:
> > > +.TP
> > > +.B LANDLOCK_ACCESS_FS_IOCTL_DEV
> > > +Invoke
> > > +.BR ioctl (2)
> > > +commands on an opened character or block device.
> > > +.IP
> > > +This access right applies to all
> > > +.BR ioctl (2)
> > > +commands implemented by device drivers.
> > > +However, the following common IOCTL commands continue to be invokable
> > 
> > Maybe s/IOCTL/ioctl(2)/ ?
> 
> ioctl(2) is already used in the previous sentence, so it might be too
> much?

I agree with Mickaël here, I feel that IOCTL might be more appropriate.

We've been using the uppercase spelling throughout the kernel docs to refer to
IOCTL (the concept and the overall feature), as opposed to ioctl(2) (the
specific system call).

This is the only remark which I left unaddressed in v2 of this patch set.
We can change it though if you feel strongly about it, Alejandro.
Otherwise I'd prefer to keep it in sync with the kernel documentation,
to keep the delta between the kernel doc and man pages smaller.


> > > @@ -440,7 +485,8 @@ attr.handled_access_fs =
> > >          LANDLOCK_ACCESS_FS_MAKE_BLOCK |
> > >          LANDLOCK_ACCESS_FS_MAKE_SYM |
> > >          LANDLOCK_ACCESS_FS_REFER |
> > > -        LANDLOCK_ACCESS_FS_TRUNCATE;
> > > +        LANDLOCK_ACCESS_FS_TRUNCATE |;
> > 
> > s/;//
> > 
> > right?
> 
> Correct

Well spotted, thanks for the attention to detail!  Done.

—Günther