Re: [PATCH v4 4/4] landlock: Document Landlock's file truncation support

["Günther Noack" <gnoack3000@gmail.com>]

On Tue, Aug 16, 2022 at 09:18:33PM +0200, Mickaël Salaün wrote:
> On 14/08/2022 21:26, Günther Noack wrote:
> > diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> > index 6648e59fabe7..3ceb97cbe9d1 100644
> > --- a/Documentation/userspace-api/landlock.rst
> > +++ b/Documentation/userspace-api/landlock.rst
> >   Because we may not know on which kernel version an application will be
> > @@ -69,16 +70,26 @@ should try to protect users as much as possible whatever the kernel they are
> >   using.  To avoid binary enforcement (i.e. either all security features or
> >   none), we can leverage a dedicated Landlock command to get the current version
> >   of the Landlock ABI and adapt the handled accesses.  Let's check if we should
> > -remove the `LANDLOCK_ACCESS_FS_REFER` access right which is only supported
> > -starting with the second version of the ABI.
> > +remove the `LANDLOCK_ACCESS_FS_REFER` and `LANDLOCK_ACCESS_FS_TRUNCATE` access
>
> s/and/or/

Done.

> > +Truncating files
> > +----------------
> > +
> > +The operations covered by `LANDLOCK_ACCESS_FS_WRITE_FILE` and
> > +`LANDLOCK_ACCESS_FS_TRUNCATE` both change the contents of a file and sometimes
> > +overlap in non-intuitive ways.  It is recommended to always specify both of
> > +these together.
> > +
> > +A particularly surprising example is :manpage:`creat(2)`.  The name suggests
> > +that this system call requires the rights to create and write files.  However,
> > +it also requires the truncate right if an existing file under the same name is
> > +already present.
> > +
> > +It should also be noted that truncating files does not necessarily require the
>
> I think "necessarily" is superfluous here.

Done.  I dropped the "obvious" too.

>
>
> > +`LANDLOCK_ACCESS_FS_WRITE_FILE` right.  Apart from the obvious
> > +:manpage:`truncate(2)` system call, this can also be done through
> > +:manpage:`open(2)` with the flags `O_RDONLY` and `O_TRUNC`.
>
> `O_RDONLY | O_TRUNC`.

Done.

> >   Compatibility
> >   =============
> > @@ -386,9 +415,8 @@ File truncation (ABI < 3)
> >   File truncation could not be denied before the third Landlock ABI, so it is
> >   always allowed when using a kernel that only supports the first or second ABI.
> > -Starting with the Landlock ABI version 3, it is now possible to securely
> > -control truncation thanks to the new `LANDLOCK_ACCESS_FS_TRUNCATE` access
> > -right.
> > +Starting with the Landlock ABI version 3, it is now possible to securely control
> > +truncation thanks to the new `LANDLOCK_ACCESS_FS_TRUNCATE` access right.
>
> This is an inconsistent hunk, patching the first patch.
>
> Please also move this "File truncation" section below the "File renaming and
> linking".

Thanks, fixed the ordering of commits and moved the truncation section
below "File Renaming and Linking".

—Günther

--