Re: [PATCH] landlock: Clarify documentation for the LANDLOCK_ACCESS_FS_REFER right

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

Hello Mickaël!

On Mon, Feb 13, 2023 at 08:14:27PM +0100, Mickaël Salaün wrote:
> Thanks for the doc improvement Günther!
> 
> On 02/02/2023 21:46, Günther Noack wrote:
> > Clarify the "refer" documentation by splitting up a big paragraph of text.
> > 
> > - Call out specifically that the denial by default applies to ABI v1 as well.
> > - Turn the three additional constraints for link/rename operations
> >    into bullet points, to give it more structure.
> > 
> > Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> > ---
> >   include/uapi/linux/landlock.h | 41 ++++++++++++++++++++++-------------
> >   1 file changed, 26 insertions(+), 15 deletions(-)
> > 
> > diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> > index f3223f96469..0cc58e0361f 100644
> > --- a/include/uapi/linux/landlock.h
> > +++ b/include/uapi/linux/landlock.h
> > @@ -130,21 +130,32 @@ struct landlock_path_beneath_attr {
> > + *   This access right is available since the second version of the Landlock
> > + *   ABI.  This is also the only access right which is always considered
> > + *   handled by any ruleset in such a way that reparenting a file hierarchy is
> > + *   always denied by default.  If left unspecified during the creation of a
> > + *   ruleset, linking and renaming files between different directories will be
> > + *   forbidden, also when done on a kernel using Landlock ABI v1.
> 
> What about:
> forbidden, which is also the case when using the first Landlock ABI.

Applied, makes sense.

> > + *   In addition to permitting this access right, the following constraints
> > + *   must hold for the access rights on the source and destination directory:
> > + *
> > + *   * The destination directory may not grant any access rights which are
> > + *     forbidden in the source directory.  Otherwise, the operation results in
> 
> "forbidden to the source file."
> 
> Indeed, the check is done according to the source file and compared to the
> potential destination file. For instance, the parent source directory may
> not allow to execute the source file, but the link will be allowed even if
> the destination directory allows file execution in the case of the execution
> right being tied to the (linked) file itself.

Ah, I didn't notice that, thanks for pointing it out! Changed.

> > + *     an ``EXDEV`` error.
> > + *
> > + *   * When linking or renaming, the ``LANDLOCK_ACCESS_FS_MAKE_*`` right for
> > + *     the respective file type is required in the destination directory.
> > + *     Otherwise, the operation results in an ``EACCES`` error.
> > + *
> > + *   * When renaming, the ``LANDLOCK_ACCESS_FS_REMOVE_*`` right for the
> > + *     respective file type is required in the source directory.  Otherwise,
> > + *     the operation results in an ``EACCES`` error.
> 
> There is also the RENAME_EXCHANGE case when we need make and remove access
> rights in the source and in the destination directory, but it would probably
> confuse readers to specify that.

I'll leave it out for now, to keep this patch focused on rewording. I
feel that adding it now would open another can of worms better left
for a more dedicated code review. ;)

–-Günther