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

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:

quoted

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 <redacted>
---
  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.

quoted

+ *   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.

quoted

+ *     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