[PATCH 1/3] landlock.7: Move the warning about missing features into the CAVEATS section

[PATCH 1/3] landlock.7: Move the warning about missing features into the CAVEATS section

[Günther Noack]

Putting the warning there makes it more prominent.
CAVEATS is a standard section that exists in many man pages
and is also described in man-pages(7).
---
 man7/landlock.7 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man7/landlock.7 b/man7/landlock.7
index 0818b4bf9..099f68067 100644
--- a/man7/landlock.7
+++ b/man7/landlock.7
@@ -201,7 +201,7 @@ command line parameter and further to the value of
 We can check that Landlock is enabled by looking for
 .I landlock: Up and running.
 in kernel logs.
-.PP
+.SH CAVEATS
 It is currently not possible to restrict some file-related actions
 accessible through these system call families:
 .BR chdir (2),

base-commit: 887b913eb2687f34bc056597c0501a4325bebf28
-- 
2.39.2

[PATCH 2/3] landlock.7: Document Landlock ABI v2 (file reparenting; kernel 5.19)

[Günther Noack]

* Add the description for LANDLOCK_ACCESS_FS_REFER,
  in line with recent update to the uapi headers:
  https://lore.kernel.org/linux-security-module/20230202204623.10345-1-gnoack3000@gmail.com/T/
* VERSIONS: Add a table of Landlock versions and their changes.
  Briefly talk about how to probe ABI levels and warn users about the
  special semantics of the LANDLOCK_ACCESS_FS_REFER right.
* Add LANDLOCK_ACCESS_FS_REFER to the code example.

Code review threads for the "refer" feature:
* https://lore.kernel.org/all/20220506161102.525323-1-mic@digikod.net/ (initial commit)
* https://lore.kernel.org/all/20220823144123.633721-1-mic@digikod.net/ (bugfix)
* https://lore.kernel.org/all/20230221165205.4231-1-gnoack3000@gmail.com/ (documentation update)
---
 man7/landlock.7 | 90 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 88 insertions(+), 2 deletions(-)

diff --git a/man7/landlock.7 b/man7/landlock.7
index 099f68067..6321b56ab 100644
--- a/man7/landlock.7
+++ b/man7/landlock.7
@@ -105,6 +105,53 @@ Create (or rename or link) a block device.
 .TP
 .B LANDLOCK_ACCESS_FS_MAKE_SYM
 Create (or rename or link) a symbolic link.
+.TP
+.B LANDLOCK_ACCESS_FS_REFER
+Link or rename a file from or to a different directory (i.e. reparent
+a file hierarchy).
+.IP
+This access right is available since the second version of the
+Landlock ABI.
+.IP
+This is the only access right which is denied by default by any
+ruleset, even if the right is not specified as handled at ruleset
+creation time.  The only way to make a ruleset grant this right is to
+explicitly allow it for a specific directory by adding a matching rule
+to the ruleset.
+.IP
+In particular, when using the first Landlock ABI version, Landlock will
+always deny attempts to reparent files between different directories.
+.IP
+In addition to the source and destination directories having the
+.B LANDLOCK_ACCESS_FS_REFER
+access right, the attempted link or rename operation must meet the
+following constraints:
+.RS
+.IP \(bu 3
+The reparented file may not gain more access rights in the destination
+directory than it previously had in the source directory.  If this is
+attempted, the operation results in an
+.B EXDEV
+error.
+.IP \(bu 3
+When linking or renaming, the
+.B LANDLOCK_ACCESS_FS_MAKE_*
+right for the respective file type must be granted for the destination
+directory. Otherwise, the operation results in an
+.BR EACCES
+error.
+.IP \(bu 3
+When renaming, the
+.B LANDLOCK_ACCESS_FS_REMOVE_*
+right for the respective file type must be granted for the source directory. Otherwise, the operation results in an
+.B EACCES
+error.
+.RE
+.IP
+If multiple requirements are not met, the
+.B EACCES
+error code takes precedence over
+.BR EXDEV .
 .\"
 .SS Layers of file path access rights
 Each time a thread enforces a ruleset on itself,
@@ -182,7 +229,45 @@ and related syscalls on a target process,
 a sandboxed process should have a subset of the target process rules,
 which means the tracee must be in a sub-domain of the tracer.
 .SH VERSIONS
-Landlock was added in Linux 5.13.
+Landlock was introduced in Linux 5.13.
+.PP
+The availability of individual Landlock features is versioned through
+ABI levels:
+.TS
+box;
+ntb| ntb| lbx
+nt| nt| lbx.
+ABI	Kernel	Newly introduced access rights
+_	_	_
+1	5.13	LANDLOCK_ACCESS_FS_EXECUTE
+\^	\^	LANDLOCK_ACCESS_FS_WRITE_FILE
+\^	\^	LANDLOCK_ACCESS_FS_READ_FILE
+\^	\^	LANDLOCK_ACCESS_FS_READ_DIR
+\^	\^	LANDLOCK_ACCESS_FS_REMOVE_DIR
+\^	\^	LANDLOCK_ACCESS_FS_REMOVE_FILE
+\^	\^	LANDLOCK_ACCESS_FS_MAKE_CHAR
+\^	\^	LANDLOCK_ACCESS_FS_MAKE_DIR
+\^	\^	LANDLOCK_ACCESS_FS_MAKE_REG
+\^	\^	LANDLOCK_ACCESS_FS_MAKE_SOCK
+\^	\^	LANDLOCK_ACCESS_FS_MAKE_FIFO
+\^	\^	LANDLOCK_ACCESS_FS_MAKE_BLOCK
+\^	\^	LANDLOCK_ACCESS_FS_MAKE_SYM
+_	_	_
+2	5.19	LANDLOCK_ACCESS_FS_REFER
+.TE
+.PP
+To query the running kernel's Landlock ABI level, programs may pass
+the
+.B LANDLOCK_CREATE_RULESET_VERSION
+flag to
+.BR landlock_create_ruleset (2).
+.PP
+When building fallback mechanisms for compatibility with older kernels,
+users are advised to consider the special semantics of the
+.B LANDLOCK_ACCESS_FS_REFER
+access right: In ABI v1, linking and moving of files between different
+directories is always forbidden, so programs relying on such
+operations are only compatible with Landlock ABI v2 and higher.
 .SH NOTES
 Landlock is enabled by
 .BR CONFIG_SECURITY_LANDLOCK .
@@ -242,7 +327,8 @@ attr.handled_access_fs =
         LANDLOCK_ACCESS_FS_MAKE_SOCK |
         LANDLOCK_ACCESS_FS_MAKE_FIFO |
         LANDLOCK_ACCESS_FS_MAKE_BLOCK |
-        LANDLOCK_ACCESS_FS_MAKE_SYM;
+        LANDLOCK_ACCESS_FS_MAKE_SYM |
+        LANDLOCK_ACCESS_FS_REFER;
 
 ruleset_fd = landlock_create_ruleset(&attr, sizeof(attr), 0);
 if (ruleset_fd == -1) {
-- 
2.39.2

[PATCH 3/3] landlock.7: Document Landlock ABI v3 (file truncation; kernel 6.2)

[Günther Noack]

https://lore.kernel.org/all/20221018182216.301684-1-gnoack3000@gmail.com/
---
 man7/landlock.7 | 80 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 77 insertions(+), 3 deletions(-)

diff --git a/man7/landlock.7 b/man7/landlock.7
index 6321b56ab..b5b356642 100644
--- a/man7/landlock.7
+++ b/man7/landlock.7
@@ -63,10 +63,38 @@ A file can only receive these access rights:
 Execute a file.
 .TP
 .B LANDLOCK_ACCESS_FS_WRITE_FILE
-Open a file with write access.
+Open a file with write access. Note that you might additionally need the
+.B LANDLOCK_ACCESS_FS_TRUNCATE
+right in order to overwrite files with
+.BR open (2)
+using
+.B O_TRUNC
+or
+.BR creat (2).
 .TP
 .B LANDLOCK_ACCESS_FS_READ_FILE
 Open a file with read access.
+.TP
+.B LANDLOCK_ACCESS_FS_TRUNCATE
+Truncate a file with
+.BR truncate (2),
+.BR ftruncate (2),
+.BR creat (2),
+or
+.BR open (2)
+with
+.BR O_TRUNC .
+Whether an opened file can be truncated with
+.BR ftruncate (2)
+is determined during
+.BR open (2),
+in the same way as read and write permissions are checked during
+.BR open (2)
+using
+.B LANDLOCK_ACCESS_FS_READ_FILE
+and
+.BR LANDLOCK_ACCESS_FS_WRITE_FILE .
+This access right is available since the third version of the Landlock ABI.
 .PP
 A directory can receive access rights related to files or directories.
 The following access right is applied to the directory itself,
@@ -228,6 +256,50 @@ To be allowed to use
 and related syscalls on a target process,
 a sandboxed process should have a subset of the target process rules,
 which means the tracee must be in a sub-domain of the tracer.
+.\"
+.SS Truncating files
+The operations covered by
+.B LANDLOCK_ACCESS_FS_WRITE_FILE
+and
+.B 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.
+.PP
+A particularly surprising example is
+.BR 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.
+.PP
+It should also be noted that truncating files does not require the
+.B LANDLOCK_ACCESS_FS_WRITE_FILE
+right.  Apart from the
+.BR truncate (2)
+system call, this can also be done through
+.BR open (2)
+with the flags
+.BR "O_RDONLY | O_TRUNC" .
+.PP
+When opening a file, the availability of the
+.B LANDLOCK_ACCESS_FS_TRUNCATE
+right is associated with the newly created file descriptor and will be used for
+subsequent truncation attempts using
+.BR ftruncate (2).
+The behavior is similar to opening a file for reading or writing,
+where permissions are checked during
+.BR open (2),
+but not during the subsequent
+.BR read (2)
+and
+.BR write (2)
+calls.
+.PP
+As a consequence, it is possible to have multiple open file descriptors for the
+same file, where one grants the right to truncate the file and the other does
+not.  It is also possible to pass such file descriptors between processes,
+keeping their Landlock properties, even when these processes do not have an
+enforced Landlock ruleset.
 .SH VERSIONS
 Landlock was introduced in Linux 5.13.
 .PP
@@ -254,6 +326,8 @@ _	_	_
 \^	\^	LANDLOCK_ACCESS_FS_MAKE_SYM
 _	_	_
 2	5.19	LANDLOCK_ACCESS_FS_REFER
+_	_	_
+3	6.2	LANDLOCK_ACCESS_FS_TRUNCATE
 .TE
 .PP
 To query the running kernel's Landlock ABI level, programs may pass
@@ -290,7 +364,6 @@ in kernel logs.
 It is currently not possible to restrict some file-related actions
 accessible through these system call families:
 .BR chdir (2),
-.BR truncate (2),
 .BR stat (2),
 .BR flock (2),
 .BR chmod (2),
@@ -328,7 +401,8 @@ attr.handled_access_fs =
         LANDLOCK_ACCESS_FS_MAKE_FIFO |
         LANDLOCK_ACCESS_FS_MAKE_BLOCK |
         LANDLOCK_ACCESS_FS_MAKE_SYM |
-        LANDLOCK_ACCESS_FS_REFER;
+        LANDLOCK_ACCESS_FS_REFER |;
+        LANDLOCK_ACCESS_FS_TRUNCATE;
 
 ruleset_fd = landlock_create_ruleset(&attr, sizeof(attr), 0);
 if (ruleset_fd == -1) {
-- 
2.39.2

Re: [PATCH 2/3] landlock.7: Document Landlock ABI v2 (file reparenting; kernel 5.19)

[Mickaël Salaün]

Thanks for this update Günther.

On 2023-02-21T21:50:22.000+01:00, Günther Noack <gnoack3000@gmail.com> wrote:
>  * Add the description for LANDLOCK_ACCESS_FS_REFER,
>   in line with recent update to the uapi headers:
>   https://lore.kernel.org/linux-security-module/20230202204623.10345-1-gnoack3000@gmail.com/T/
> * VERSIONS: Add a table of Landlock versions and their changes.
>   Briefly talk about how to probe ABI levels and warn users about the
>   special semantics of the LANDLOCK_ACCESS_FS_REFER right.
> * Add LANDLOCK_ACCESS_FS_REFER to the code example.
> 
> Code review threads for the "refer" feature:
> * https://lore.kernel.org/all/20220506161102.525323-1-mic@digikod.net/ (initial commit)
> * https://lore.kernel.org/all/20220823144123.633721-1-mic@digikod.net/ (bugfix)
> * https://lore.kernel.org/all/20230221165205.4231-1-gnoack3000@gmail.com/ (documentation update)
> ---
>  man7/landlock.7 | 90 +++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 88 insertions(+), 2 deletions(-)
> 
> diff --git a/man7/landlock.7 b/man7/landlock.7
> index 099f68067..6321b56ab 100644
> --- a/man7/landlock.7
> +++ b/man7/landlock.7
> @@ -105,6 +105,53 @@ Create (or rename or link) a block device.
>  .TP
>  .B LANDLOCK_ACCESS_FS_MAKE_SYM
>  Create (or rename or link) a symbolic link.
> +.TP
> +.B LANDLOCK_ACCESS_FS_REFER
> +Link or rename a file from or to a different directory (i.e. reparent
> +a file hierarchy).
> +.IP
> +This access right is available since the second version of the
> +Landlock ABI.
> +.IP
> +This is the only access right which is denied by default by any
> +ruleset, even if the right is not specified as handled at ruleset
> +creation time.  The only way to make a ruleset grant this right is to
> +explicitly allow it for a specific directory by adding a matching rule
> +to the ruleset.
> +.IP
> +In particular, when using the first Landlock ABI version, Landlock will
> +always deny attempts to reparent files between different directories.
> +.IP
> +In addition to the source and destination directories having the
> +.B LANDLOCK_ACCESS_FS_REFER
> +access right, the attempted link or rename operation must meet the
> +following constraints:
> +.RS
> +.IP \(bu 3
> +The reparented file may not gain more access rights in the destination
> +directory than it previously had in the source directory.  If this is
> +attempted, the operation results in an
> +.B EXDEV
> +error.
> +.IP \(bu 3
> +When linking or renaming, the
> +.B LANDLOCK_ACCESS_FS_MAKE_*
> +right for the respective file type must be granted for the destination
> +directory. Otherwise, the operation results in an
> +.BR EACCES
> +error.
> +.IP \(bu 3
> +When renaming, the
> +.B LANDLOCK_ACCESS_FS_REMOVE_*
> +right for the respective file type must be granted for the source directory. Otherwise, the operation results in an
> +.B EACCES
> +error.
> +.RE
> +.IP
> +If multiple requirements are not met, the
> +.B EACCES
> +error code takes precedence over
> +.BR EXDEV .
>  .\"
>  .SS Layers of file path access rights
>  Each time a thread enforces a ruleset on itself,
> @@ -182,7 +229,45 @@ and related syscalls on a target process,
>  a sandboxed process should have a subset of the target process rules,
>  which means the tracee must be in a sub-domain of the tracer.
>  .SH VERSIONS
> -Landlock was added in Linux 5.13.
> +Landlock was introduced in Linux 5.13.
> +.PP
> +The availability of individual Landlock features is versioned through
> +ABI levels:

I think this table is useful, but it should contain a warning to make sure developers don't rely on kernel versions to check Landlock features, but use the dedicated Landlock syscall instead.
It should be explained that this table is true for the mainline/vanilla kernel, but that can be incorrect for any other kernel (e.g. patched distro kernel, like chromeOS that may backport upstream features).

> +.TS
> +box;
> +ntb| ntb| lbx
> +nt| nt| lbx.
> +ABI	Kernel	Newly introduced access rights
> +_	_	_
> +1	5.13	LANDLOCK_ACCESS_FS_EXECUTE
> +\^	\^	LANDLOCK_ACCESS_FS_WRITE_FILE
> +\^	\^	LANDLOCK_ACCESS_FS_READ_FILE
> +\^	\^	LANDLOCK_ACCESS_FS_READ_DIR
> +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_DIR
> +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_FILE
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_CHAR
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_DIR
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_REG
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SOCK
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_FIFO
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_BLOCK
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SYM
> +_	_	_
> +2	5.19	LANDLOCK_ACCESS_FS_REFER
> +.TE
> +.PP
> +To query the running kernel's Landlock ABI level, programs may pass
> +the
> +.B LANDLOCK_CREATE_RULESET_VERSION
> +flag to
> +.BR landlock_create_ruleset (2).
> +.PP
> +When building fallback mechanisms for compatibility with older kernels,
> +users are advised to consider the special semantics of the
> +.B LANDLOCK_ACCESS_FS_REFER
> +access right: In ABI v1, linking and moving of files between different
> +directories is always forbidden, so programs relying on such
> +operations are only compatible with Landlock ABI v2 and higher.
>  .SH NOTES
>  Landlock is enabled by
>  .BR CONFIG_SECURITY_LANDLOCK .
> @@ -242,7 +327,8 @@ attr.handled_access_fs =
>          LANDLOCK_ACCESS_FS_MAKE_SOCK |
>          LANDLOCK_ACCESS_FS_MAKE_FIFO |
>          LANDLOCK_ACCESS_FS_MAKE_BLOCK |
> -        LANDLOCK_ACCESS_FS_MAKE_SYM;
> +        LANDLOCK_ACCESS_FS_MAKE_SYM |
> +        LANDLOCK_ACCESS_FS_REFER;
>  
>  ruleset_fd = landlock_create_ruleset(&attr, sizeof(attr), 0);
>  if (ruleset_fd == -1) {
> -- 
2.39.2

Re: [PATCH 2/3] landlock.7: Document Landlock ABI v2 (file reparenting; kernel 5.19)

[Mickaël Salaün]

On 2023-02-21T21:50:22.000+01:00, Günther Noack wrote:

[...]

>  .SH VERSIONS
> -Landlock was added in Linux 5.13.
> +Landlock was introduced in Linux 5.13.
> +.PP
> +The availability of individual Landlock features is versioned through
> +ABI levels:
> +.TS
> +box;
> +ntb| ntb| lbx
> +nt| nt| lbx.
> +ABI	Kernel	Newly introduced access rights
> +_	_	_
> +1	5.13	LANDLOCK_ACCESS_FS_EXECUTE
> +\^	\^	LANDLOCK_ACCESS_FS_WRITE_FILE
> +\^	\^	LANDLOCK_ACCESS_FS_READ_FILE
> +\^	\^	LANDLOCK_ACCESS_FS_READ_DIR
> +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_DIR
> +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_FILE
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_CHAR
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_DIR
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_REG
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SOCK
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_FIFO
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_BLOCK
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SYM
> +_	_	_
> +2	5.19	LANDLOCK_ACCESS_FS_REFER
> +.TE
> +.PP

A line break would be nice here.

> +To query the running kernel's Landlock ABI level, programs may pass

s/level/version/

> +the
> +.B LANDLOCK_CREATE_RULESET_VERSION
> +flag to
> +.BR landlock_create_ruleset (2).

Re: [PATCH 2/3] landlock.7: Document Landlock ABI v2 (file reparenting; kernel 5.19)

[Mickaël Salaün]

On 2023-02-21T21:50:22.000+01:00, Günther Noack wrote:

[...]

>  .SH VERSIONS
> -Landlock was added in Linux 5.13.
> +Landlock was introduced in Linux 5.13.
> +.PP
> +The availability of individual Landlock features is versioned through
> +ABI levels:
> +.TS
> +box;
> +ntb| ntb| lbx
> +nt| nt| lbx.
> +ABI	Kernel	Newly introduced access rights
> +_	_	_
> +1	5.13	LANDLOCK_ACCESS_FS_EXECUTE
> +\^	\^	LANDLOCK_ACCESS_FS_WRITE_FILE
> +\^	\^	LANDLOCK_ACCESS_FS_READ_FILE
> +\^	\^	LANDLOCK_ACCESS_FS_READ_DIR
> +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_DIR
> +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_FILE
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_CHAR
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_DIR
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_REG
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SOCK
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_FIFO
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_BLOCK
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SYM
> +_	_	_
> +2	5.19	LANDLOCK_ACCESS_FS_REFER
> +.TE
> +.PP
> +To query the running kernel's Landlock ABI level, programs may pass
> +the
> +.B LANDLOCK_CREATE_RULESET_VERSION
> +flag to
> +.BR landlock_create_ruleset (2).
> +.PP
> +When building fallback mechanisms for compatibility with older kernels,
> +users are advised to consider the special semantics of the
> +.B LANDLOCK_ACCESS_FS_REFER
> +access right: In ABI v1, linking and moving of files between different
> +directories is always forbidden, so programs relying on such
> +operations are only compatible with Landlock ABI v2 and higher.
>  .SH NOTES
>  Landlock is enabled by
>  .BR CONFIG_SECURITY_LANDLOCK .
> @@ -242,7 +327,8 @@ attr.handled_access_fs =
>          LANDLOCK_ACCESS_FS_MAKE_SOCK |
>          LANDLOCK_ACCESS_FS_MAKE_FIFO |
>          LANDLOCK_ACCESS_FS_MAKE_BLOCK |
> -        LANDLOCK_ACCESS_FS_MAKE_SYM;
> +        LANDLOCK_ACCESS_FS_MAKE_SYM |
> +        LANDLOCK_ACCESS_FS_REFER;

This code example should now query the Landlock ABI version and mask new access right to make it works with old kernels.

>  
>  ruleset_fd = landlock_create_ruleset(&attr, sizeof(attr), 0);
>  if (ruleset_fd == -1) {
> -- 
2.39.2

Re: [PATCH 3/3] landlock.7: Document Landlock ABI v3 (file truncation; kernel 6.2)

[Mickaël Salaün]

On 2023-02-21T21:50:23.000+01:00, Günther Noack wrote:
>  https://lore.kernel.org/all/20221018182216.301684-1-gnoack3000@gmail.com/

We should point to the git (merge) commit once it is merged in the kernel:
https://git.kernel.org/torvalds/c/299e2b1967578b1442128ba8b3e86ed3427d3651

[...]

> @@ -328,7 +401,8 @@ attr.handled_access_fs =
>          LANDLOCK_ACCESS_FS_MAKE_FIFO |
>          LANDLOCK_ACCESS_FS_MAKE_BLOCK |
>          LANDLOCK_ACCESS_FS_MAKE_SYM |
> -        LANDLOCK_ACCESS_FS_REFER;
> +        LANDLOCK_ACCESS_FS_REFER |;

trailing ";"

> +        LANDLOCK_ACCESS_FS_TRUNCATE;
>  
>  ruleset_fd = landlock_create_ruleset(&attr, sizeof(attr), 0);
>  if (ruleset_fd == -1) {
> -- 
2.39.2

Re: [PATCH 2/3] landlock.7: Document Landlock ABI v2 (file reparenting; kernel 5.19)

[Günther Noack]

On Wed, Feb 22, 2023 at 08:01:31AM +0100, Mickaël Salaün wrote:
> I think this table is useful, but it should contain a warning to make sure developers don't rely on kernel versions to check Landlock features, but use the dedicated Landlock syscall instead.
> It should be explained that this table is true for the mainline/vanilla kernel, but that can be incorrect for any other kernel (e.g. patched distro kernel, like chromeOS that may backport upstream features).

Thanks, that's a good point. I'll add that.

Re: [PATCH 2/3] landlock.7: Document Landlock ABI v2 (file reparenting; kernel 5.19)

[Günther Noack]

On Wed, Feb 22, 2023 at 08:36:37AM +0100, Mickaël Salaün wrote:
> On 2023-02-21T21:50:22.000+01:00, Günther Noack wrote:
> > +The availability of individual Landlock features is versioned through
> > +ABI levels:
> > +.TS
> > +box;
> > +ntb| ntb| lbx
> > +nt| nt| lbx.
> > +ABI	Kernel	Newly introduced access rights
> > +_	_	_
> > +1	5.13	LANDLOCK_ACCESS_FS_EXECUTE
> > +\^	\^	LANDLOCK_ACCESS_FS_WRITE_FILE
> > +\^	\^	LANDLOCK_ACCESS_FS_READ_FILE
> > +\^	\^	LANDLOCK_ACCESS_FS_READ_DIR
> > +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_DIR
> > +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_FILE
> > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_CHAR
> > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_DIR
> > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_REG
> > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SOCK
> > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_FIFO
> > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_BLOCK
> > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SYM
> > +_	_	_
> > +2	5.19	LANDLOCK_ACCESS_FS_REFER
> > +.TE
> > +.PP
> 
> A line break would be nice here.

Added. (Used .sp 1 for that, as it is already used in the
mount_namespaces.7, ip.7 and other man pages.)

> > +To query the running kernel's Landlock ABI level, programs may pass
> 
> s/level/version/

Thanks, I'm removing the word "level" here.

Re: [PATCH 2/3] landlock.7: Document Landlock ABI v2 (file reparenting; kernel 5.19)

[Günther Noack]

On Wed, Feb 22, 2023 at 08:45:28AM +0100, Mickaël Salaün wrote:
> On 2023-02-21T21:50:22.000+01:00, Günther Noack wrote:
> > @@ -242,7 +327,8 @@ attr.handled_access_fs =
> >          LANDLOCK_ACCESS_FS_MAKE_SOCK |
> >          LANDLOCK_ACCESS_FS_MAKE_FIFO |
> >          LANDLOCK_ACCESS_FS_MAKE_BLOCK |
> > -        LANDLOCK_ACCESS_FS_MAKE_SYM;
> > +        LANDLOCK_ACCESS_FS_MAKE_SYM |
> > +        LANDLOCK_ACCESS_FS_REFER;
> 
> This code example should now query the Landlock ABI version and mask new access right to make it works with old kernels.

The changes I would have to do are:

- I would *remove* LANDLOCK_ACCESS_FS_REFER from
  attr.handled_access_fs, because it is not used in the
  LANDLOCK_RULE_PATH_PENEATH and that right is implicit anyway.

- Patch 3/3: I would *keep* LANDLOCK_ACCESS_FS_TRUNCATE in
  attr.handled_access_fs.

- Patch 3/3: I would query the ABI version, and mask away the
  LANDLOCK_ACCESS_FS_TRUNCATE right in handled_access_fs if the ABI
  version is < 3.

Things I don't like yet about this approach are:

* I believe if I were to read that example for the first time, I would
  be puzzled when seeing the "truncate" right set in
  handled_access_fs, but the "refer" right omitted. This requires
  additional explanation.

* It does not really describe what to do if you actually need to
  reparent files with the "refer" right in your program. The fallback
  logic is simpler in the man page example because "refer" is not
  needed in the LANDLOCK_RULE_PATH_BENEATH rule.

  At the same time, I feel that the more complicated "refer" fallback
  logic might be beyond what would reasonably fit into a man page.
  Especially given that many users probably don't need the "refer"
  right for their programs.

So my proposal would be to change the example as described in the
three bullet points above, but to point out that users who need
"refer" should refer (ha ha) to the kernel documentation for more
details.

Does that sound reasonable to you?

–-Günther

Re: [PATCH 3/3] landlock.7: Document Landlock ABI v3 (file truncation; kernel 6.2)

[Günther Noack]

On Wed, Feb 22, 2023 at 09:04:00AM +0100, Mickaël Salaün wrote:
> On 2023-02-21T21:50:23.000+01:00, Günther Noack wrote:
> >  https://lore.kernel.org/all/20221018182216.301684-1-gnoack3000@gmail.com/
> We should point to the git (merge) commit once it is merged in the kernel:
> https://git.kernel.org/torvalds/c/299e2b1967578b1442128ba8b3e86ed3427d3651

Ah, thanks. Will change it in the next patch version.

> > @@ -328,7 +401,8 @@ attr.handled_access_fs =
> >          LANDLOCK_ACCESS_FS_MAKE_FIFO |
> >          LANDLOCK_ACCESS_FS_MAKE_BLOCK |
> >          LANDLOCK_ACCESS_FS_MAKE_SYM |
> > -        LANDLOCK_ACCESS_FS_REFER;
> > +        LANDLOCK_ACCESS_FS_REFER |;
> 
> trailing ";"

Oops. Done.

Re: [PATCH 1/3] landlock.7: Move the warning about missing features into the CAVEATS section

[Alex Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 1006 bytes --]

Hi Günther,

On 2/21/23 21:50, Günther Noack wrote:
> Putting the warning there makes it more prominent.
> CAVEATS is a standard section that exists in many man pages
> and is also described in man-pages(7).

Patch applied.

Thanks,

Alex

> ---
>   man7/landlock.7 | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/man7/landlock.7 b/man7/landlock.7
> index 0818b4bf9..099f68067 100644
> --- a/man7/landlock.7
> +++ b/man7/landlock.7
> @@ -201,7 +201,7 @@ command line parameter and further to the value of
>   We can check that Landlock is enabled by looking for
>   .I landlock: Up and running.
>   in kernel logs.
> -.PP
> +.SH CAVEATS
>   It is currently not possible to restrict some file-related actions
>   accessible through these system call families:
>   .BR chdir (2),
> 
> base-commit: 887b913eb2687f34bc056597c0501a4325bebf28

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


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

Re: [PATCH 2/3] landlock.7: Document Landlock ABI v2 (file reparenting; kernel 5.19)

[Alex Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 7399 bytes --]

Hi Günther,

On 2/21/23 21:50, Günther Noack wrote:
> * Add the description for LANDLOCK_ACCESS_FS_REFER,
>    in line with recent update to the uapi headers:
>    https://lore.kernel.org/linux-security-module/20230202204623.10345-1-gnoack3000@gmail.com/T/
> * VERSIONS: Add a table of Landlock versions and their changes.
>    Briefly talk about how to probe ABI levels and warn users about the
>    special semantics of the LANDLOCK_ACCESS_FS_REFER right.
> * Add LANDLOCK_ACCESS_FS_REFER to the code example.
> 
> Code review threads for the "refer" feature:
> * https://lore.kernel.org/all/20220506161102.525323-1-mic@digikod.net/ (initial commit)
> * https://lore.kernel.org/all/20220823144123.633721-1-mic@digikod.net/ (bugfix)
> * https://lore.kernel.org/all/20230221165205.4231-1-gnoack3000@gmail.com/ (documentation update)

I finally found some time to review your work.  Thanks for the patience!

Please see some comments below.

Thanks!

Alex

> ---
>   man7/landlock.7 | 90 +++++++++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 88 insertions(+), 2 deletions(-)
> 
> diff --git a/man7/landlock.7 b/man7/landlock.7
> index 099f68067..6321b56ab 100644
> --- a/man7/landlock.7
> +++ b/man7/landlock.7
> @@ -105,6 +105,53 @@ Create (or rename or link) a block device.
>   .TP
>   .B LANDLOCK_ACCESS_FS_MAKE_SYM
>   Create (or rename or link) a symbolic link.
> +.TP
> +.B LANDLOCK_ACCESS_FS_REFER
> +Link or rename a file from or to a different directory (i.e. reparent
> +a file hierarchy).

Please have a look at man-pages(7):
    Use semantic newlines
        In the source of a manual page, new sentences should be started
        on  new  lines,  long  sentences  should be split into lines at
        clause breaks (commas, semicolons, colons, and so on), and long
        clauses should be split at phrase boundaries.  This convention,
        sometimes known as "semantic newlines", makes it easier to  see
        the  effect of patches, which often operate at the level of in‐
        dividual sentences, clauses, or phrases.

Here,
that would mean
breaking the line right before the opening parenthesis;
please also apply it to the rest of the patch where appropriate.


> +.IP
> +This access right is available since the second version of the
> +Landlock ABI.
> +.IP
> +This is the only access right which is denied by default by any
> +ruleset, even if the right is not specified as handled at ruleset
> +creation time.  The only way to make a ruleset grant this right is to
> +explicitly allow it for a specific directory by adding a matching rule
> +to the ruleset.
> +.IP
> +In particular, when using the first Landlock ABI version, Landlock will
> +always deny attempts to reparent files between different directories.
> +.IP
> +In addition to the source and destination directories having the
> +.B LANDLOCK_ACCESS_FS_REFER
> +access right, the attempted link or rename operation must meet the
> +following constraints:
> +.RS
> +.IP \(bu 3

We now use \[bu] instead of \(bu (they are equivalent).

commit 36f73ba37945c7ff4aa2d8383f831519a38e3f27
Author: Alejandro Colomar <alx@kernel.org>
Date:   Sun Feb 5 22:59:22 2023 +0100

     man-pages.7: Recommend using \[..] instead of \(.. escapes

     They are more readable.

     Signed-off-by: Alejandro Colomar <alx@kernel.org>

diff --git a/man7/man-pages.7 b/man7/man-pages.7
index 4132ddabe..e5b111283 100644
--- a/man7/man-pages.7
+++ b/man7/man-pages.7
@@ -666,7 +666,7 @@ These represent a set of (normally) exclusive 
alternatives.
  .TP
  Bullet lists
  Elements are preceded by bullet symbols
-.RB ( \e(bu ).
+.RB ( \e[bu] ).
  Anything that doesn't fit elsewhere is
  usually covered by this type of list.
  .TP
@@
[...]

commit cdede5cdd1b0ba75135d3b32d96354026e96f866
Author: Alejandro Colomar <alx@kernel.org>
Date:   Sun Feb 5 23:14:38 2023 +0100

     Many pages: Use \[bu] instead of \(bu

     Signed-off-by: Alejandro Colomar <alx@kernel.org>



> +The reparented file may not gain more access rights in the destination
> +directory than it previously had in the source directory.  If this is
> +attempted, the operation results in an
> +.B EXDEV
> +error.
> +.IP \(bu 3
> +When linking or renaming, the
> +.B LANDLOCK_ACCESS_FS_MAKE_*
> +right for the respective file type must be granted for the destination
> +directory. Otherwise, the operation results in an
> +.BR EACCES
> +error.
> +.IP \(bu 3
> +When renaming, the
> +.B LANDLOCK_ACCESS_FS_REMOVE_*
> +right for the respective file type must be granted for the source directory. Otherwise, the operation results in an

80 columns is a strong limit.
Using semantic newlines as suggested above should fix this.

> +.B EACCES
> +error.
> +.RE
> +.IP
> +If multiple requirements are not met, the
> +.B EACCES
> +error code takes precedence over
> +.BR EXDEV .
>   .\"
>   .SS Layers of file path access rights
>   Each time a thread enforces a ruleset on itself,
> @@ -182,7 +229,45 @@ and related syscalls on a target process,
>   a sandboxed process should have a subset of the target process rules,
>   which means the tracee must be in a sub-domain of the tracer.
>   .SH VERSIONS
> -Landlock was added in Linux 5.13.
> +Landlock was introduced in Linux 5.13.
> +.PP
> +The availability of individual Landlock features is versioned through
> +ABI levels:
> +.TS
> +box;
> +ntb| ntb| lbx
> +nt| nt| lbx.
> +ABI	Kernel	Newly introduced access rights
> +_	_	_
> +1	5.13	LANDLOCK_ACCESS_FS_EXECUTE
> +\^	\^	LANDLOCK_ACCESS_FS_WRITE_FILE

What character do you want here?
If you want ASCII 0x5E,
then you want to use \[ha].

> +\^	\^	LANDLOCK_ACCESS_FS_READ_FILE
> +\^	\^	LANDLOCK_ACCESS_FS_READ_DIR
> +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_DIR
> +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_FILE
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_CHAR
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_DIR
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_REG
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SOCK
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_FIFO
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_BLOCK
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SYM
> +_	_	_
> +2	5.19	LANDLOCK_ACCESS_FS_REFER
> +.TE
> +.PP
> +To query the running kernel's Landlock ABI level, programs may pass
> +the
> +.B LANDLOCK_CREATE_RULESET_VERSION
> +flag to
> +.BR landlock_create_ruleset (2).
> +.PP
> +When building fallback mechanisms for compatibility with older kernels,
> +users are advised to consider the special semantics of the
> +.B LANDLOCK_ACCESS_FS_REFER
> +access right: In ABI v1, linking and moving of files between different
> +directories is always forbidden, so programs relying on such
> +operations are only compatible with Landlock ABI v2 and higher.
>   .SH NOTES
>   Landlock is enabled by
>   .BR CONFIG_SECURITY_LANDLOCK .
> @@ -242,7 +327,8 @@ attr.handled_access_fs =
>           LANDLOCK_ACCESS_FS_MAKE_SOCK |
>           LANDLOCK_ACCESS_FS_MAKE_FIFO |
>           LANDLOCK_ACCESS_FS_MAKE_BLOCK |
> -        LANDLOCK_ACCESS_FS_MAKE_SYM;
> +        LANDLOCK_ACCESS_FS_MAKE_SYM |
> +        LANDLOCK_ACCESS_FS_REFER;
>   
>   ruleset_fd = landlock_create_ruleset(&attr, sizeof(attr), 0);
>   if (ruleset_fd == -1) {

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


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

Re: [PATCH 3/3] landlock.7: Document Landlock ABI v3 (file truncation; kernel 6.2)

[Alex Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 5056 bytes --]

Hi Günther,

On 2/21/23 21:50, Günther Noack wrote:
> https://lore.kernel.org/all/20221018182216.301684-1-gnoack3000@gmail.com/

Please see some comments below.

Cheers,

Alex

> ---
>   man7/landlock.7 | 80 +++++++++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 77 insertions(+), 3 deletions(-)
> 
> diff --git a/man7/landlock.7 b/man7/landlock.7
> index 6321b56ab..b5b356642 100644
> --- a/man7/landlock.7
> +++ b/man7/landlock.7
> @@ -63,10 +63,38 @@ A file can only receive these access rights:
>   Execute a file.
>   .TP
>   .B LANDLOCK_ACCESS_FS_WRITE_FILE
> -Open a file with write access.
> +Open a file with write access. Note that you might additionally need the

Again,
please use semantic newlines.

Also,
roff(7) requires two spaces after period in source code,
to detect sentence endings.
I say this as a curiosity,
since due to the requirement of semantic newlines,
we always write a newline after period.

> +.B LANDLOCK_ACCESS_FS_TRUNCATE
> +right in order to overwrite files with
> +.BR open (2)
> +using
> +.B O_TRUNC
> +or
> +.BR creat (2).
>   .TP
>   .B LANDLOCK_ACCESS_FS_READ_FILE
>   Open a file with read access.
> +.TP
> +.B LANDLOCK_ACCESS_FS_TRUNCATE
> +Truncate a file with
> +.BR truncate (2),
> +.BR ftruncate (2),
> +.BR creat (2),
> +or
> +.BR open (2)
> +with
> +.BR O_TRUNC .
> +Whether an opened file can be truncated with
> +.BR ftruncate (2)
> +is determined during
> +.BR open (2),
> +in the same way as read and write permissions are checked during
> +.BR open (2)
> +using
> +.B LANDLOCK_ACCESS_FS_READ_FILE
> +and
> +.BR LANDLOCK_ACCESS_FS_WRITE_FILE .
> +This access right is available since the third version of the Landlock ABI.

Maybe it's simpler to say Landlock ABI v3?  That's usual convention in 
software, so not using formal English for that is not a crime :).

>   .PP
>   A directory can receive access rights related to files or directories.
>   The following access right is applied to the directory itself,
> @@ -228,6 +256,50 @@ To be allowed to use
>   and related syscalls on a target process,
>   a sandboxed process should have a subset of the target process rules,
>   which means the tracee must be in a sub-domain of the tracer.
> +.\"
> +.SS Truncating files
> +The operations covered by
> +.B LANDLOCK_ACCESS_FS_WRITE_FILE
> +and
> +.B 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.
> +.PP
> +A particularly surprising example is
> +.BR 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.
> +.PP
> +It should also be noted that truncating files does not require the
> +.B LANDLOCK_ACCESS_FS_WRITE_FILE
> +right.  Apart from the
> +.BR truncate (2)
> +system call, this can also be done through
> +.BR open (2)
> +with the flags
> +.BR "O_RDONLY | O_TRUNC" .
> +.PP
> +When opening a file, the availability of the
> +.B LANDLOCK_ACCESS_FS_TRUNCATE
> +right is associated with the newly created file descriptor and will be used for
> +subsequent truncation attempts using
> +.BR ftruncate (2).
> +The behavior is similar to opening a file for reading or writing,
> +where permissions are checked during
> +.BR open (2),
> +but not during the subsequent
> +.BR read (2)
> +and
> +.BR write (2)
> +calls.
> +.PP
> +As a consequence, it is possible to have multiple open file descriptors for the
> +same file, where one grants the right to truncate the file and the other does
> +not.  It is also possible to pass such file descriptors between processes,
> +keeping their Landlock properties, even when these processes do not have an
> +enforced Landlock ruleset.
>   .SH VERSIONS
>   Landlock was introduced in Linux 5.13.
>   .PP
> @@ -254,6 +326,8 @@ _	_	_
>   \^	\^	LANDLOCK_ACCESS_FS_MAKE_SYM
>   _	_	_
>   2	5.19	LANDLOCK_ACCESS_FS_REFER
> +_	_	_
> +3	6.2	LANDLOCK_ACCESS_FS_TRUNCATE
>   .TE
>   .PP
>   To query the running kernel's Landlock ABI level, programs may pass
> @@ -290,7 +364,6 @@ in kernel logs.
>   It is currently not possible to restrict some file-related actions
>   accessible through these system call families:
>   .BR chdir (2),
> -.BR truncate (2),
>   .BR stat (2),
>   .BR flock (2),
>   .BR chmod (2),
> @@ -328,7 +401,8 @@ attr.handled_access_fs =
>           LANDLOCK_ACCESS_FS_MAKE_FIFO |
>           LANDLOCK_ACCESS_FS_MAKE_BLOCK |
>           LANDLOCK_ACCESS_FS_MAKE_SYM |
> -        LANDLOCK_ACCESS_FS_REFER;
> +        LANDLOCK_ACCESS_FS_REFER |;
> +        LANDLOCK_ACCESS_FS_TRUNCATE;
>   
>   ruleset_fd = landlock_create_ruleset(&attr, sizeof(attr), 0);
>   if (ruleset_fd == -1) {

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


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

Re: [PATCH 2/3] landlock.7: Document Landlock ABI v2 (file reparenting; kernel 5.19)

[Alex Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 1610 bytes --]

Hi Mickaël,

On 2/22/23 08:36, Mickaël Salaün wrote:
> On 2023-02-21T21:50:22.000+01:00, Günther Noack wrote:
> 
> [...]
> 
>>   .SH VERSIONS
>> -Landlock was added in Linux 5.13.
>> +Landlock was introduced in Linux 5.13.
>> +.PP
>> +The availability of individual Landlock features is versioned through
>> +ABI levels:
>> +.TS
>> +box;
>> +ntb| ntb| lbx
>> +nt| nt| lbx.
>> +ABI	Kernel	Newly introduced access rights
>> +_	_	_
>> +1	5.13	LANDLOCK_ACCESS_FS_EXECUTE
>> +\^	\^	LANDLOCK_ACCESS_FS_WRITE_FILE
>> +\^	\^	LANDLOCK_ACCESS_FS_READ_FILE
>> +\^	\^	LANDLOCK_ACCESS_FS_READ_DIR
>> +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_DIR
>> +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_FILE
>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_CHAR
>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_DIR
>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_REG
>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SOCK
>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_FIFO
>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_BLOCK
>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SYM
>> +_	_	_
>> +2	5.19	LANDLOCK_ACCESS_FS_REFER
>> +.TE
>> +.PP
> 
> A line break would be nice here.

.PP should be performing a line break.  I didn't yet read the formatted 
version, so maybe there's something wrong with the table, but from 
inspection of the source it seems reasonable to me.

Cheers,

Alex

> 
>> +To query the running kernel's Landlock ABI level, programs may pass
> 
> s/level/version/
> 
>> +the
>> +.B LANDLOCK_CREATE_RULESET_VERSION
>> +flag to
>> +.BR landlock_create_ruleset (2).

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


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

Re: [PATCH 2/3] landlock.7: Document Landlock ABI v2 (file reparenting; kernel 5.19)

[Alex Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 3963 bytes --]

Hi Branden,

On 2/23/23 09:48, Günther Noack wrote:
> On Wed, Feb 22, 2023 at 08:36:37AM +0100, Mickaël Salaün wrote:
>> On 2023-02-21T21:50:22.000+01:00, Günther Noack wrote:
>>> +The availability of individual Landlock features is versioned through
>>> +ABI levels:
>>> +.TS
>>> +box;
>>> +ntb| ntb| lbx
>>> +nt| nt| lbx.
>>> +ABI	Kernel	Newly introduced access rights
>>> +_	_	_
>>> +1	5.13	LANDLOCK_ACCESS_FS_EXECUTE
>>> +\^	\^	LANDLOCK_ACCESS_FS_WRITE_FILE
>>> +\^	\^	LANDLOCK_ACCESS_FS_READ_FILE
>>> +\^	\^	LANDLOCK_ACCESS_FS_READ_DIR
>>> +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_DIR
>>> +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_FILE
>>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_CHAR
>>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_DIR
>>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_REG
>>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SOCK
>>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_FIFO
>>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_BLOCK
>>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SYM
>>> +_	_	_
>>> +2	5.19	LANDLOCK_ACCESS_FS_REFER
>>> +.TE
>>> +.PP
>>
>> A line break would be nice here.
> 
> Added. (Used .sp 1 for that, as it is already used in the
> mount_namespaces.7, ip.7 and other man pages.)

This sounds weird, but they are right that there seems to be a missing 
blank line.  Could you explain why it's happening?  I'd expect the .PP 
to separate paragraphs with a blank, right?  I see:


        The  availability  of individual Landlock features is versioned
        through ABI levels:

        ┌────┬────────┬────────────────────────────────────────────────┐
        │ABI │ Kernel │ Newly introduced access rights                 │
        ├────┼────────┼────────────────────────────────────────────────┤
        │ 1  │  5.13  │ LANDLOCK_ACCESS_FS_EXECUTE                     │
        │    │        │ LANDLOCK_ACCESS_FS_WRITE_FILE                  │
        │    │        │ LANDLOCK_ACCESS_FS_READ_FILE                   │
        │    │        │ LANDLOCK_ACCESS_FS_READ_DIR                    │
        │    │        │ LANDLOCK_ACCESS_FS_REMOVE_DIR                  │
        │    │        │ LANDLOCK_ACCESS_FS_REMOVE_FILE                 │
        │    │        │ LANDLOCK_ACCESS_FS_MAKE_CHAR                   │
        │    │        │ LANDLOCK_ACCESS_FS_MAKE_DIR                    │
        │    │        │ LANDLOCK_ACCESS_FS_MAKE_REG                    │
        │    │        │ LANDLOCK_ACCESS_FS_MAKE_SOCK                   │
        │    │        │ LANDLOCK_ACCESS_FS_MAKE_FIFO                   │
        │    │        │ LANDLOCK_ACCESS_FS_MAKE_BLOCK                  │
        │    │        │ LANDLOCK_ACCESS_FS_MAKE_SYM                    │
        ├────┼────────┼────────────────────────────────────────────────┤
        │ 2  │  5.19  │ LANDLOCK_ACCESS_FS_REFER                       │
        └────┴────────┴────────────────────────────────────────────────┘
        To query the running kernel's Landlock ABI level, programs  may
        pass  the LANDLOCK_CREATE_RULESET_VERSION flag to landlock_cre‐
        ate_ruleset(2).


Cheers,

Alex

> 
>>> +To query the running kernel's Landlock ABI level, programs may pass
>>
>> s/level/version/
> 
> Thanks, I'm removing the word "level" here.

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


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

Re: [PATCH 2/3] landlock.7: Document Landlock ABI v2 (file reparenting; kernel 5.19)

[G. Branden Robinson]

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

Hi Alex,

At 2023-02-25T02:10:22+0100, Alex Colomar wrote:
> On 2/23/23 09:48, Günther Noack wrote:
> > On Wed, Feb 22, 2023 at 08:36:37AM +0100, Mickaël Salaün wrote:
> > > On 2023-02-21T21:50:22.000+01:00, Günther Noack wrote:
> > > > +The availability of individual Landlock features is versioned through
> > > > +ABI levels:
> > > > +.TS
> > > > +box;
> > > > +ntb| ntb| lbx
> > > > +nt| nt| lbx.
> > > > +ABI	Kernel	Newly introduced access rights
> > > > +_	_	_
> > > > +1	5.13	LANDLOCK_ACCESS_FS_EXECUTE
> > > > +\^	\^	LANDLOCK_ACCESS_FS_WRITE_FILE
> > > > +\^	\^	LANDLOCK_ACCESS_FS_READ_FILE
> > > > +\^	\^	LANDLOCK_ACCESS_FS_READ_DIR
> > > > +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_DIR
> > > > +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_FILE
> > > > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_CHAR
> > > > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_DIR
> > > > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_REG
> > > > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SOCK
> > > > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_FIFO
> > > > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_BLOCK
> > > > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SYM
> > > > +_	_	_
> > > > +2	5.19	LANDLOCK_ACCESS_FS_REFER
> > > > +.TE
> > > > +.PP
> > > 
> > > A line break would be nice here.
> > 
> > Added. (Used .sp 1 for that, as it is already used in the
> > mount_namespaces.7, ip.7 and other man pages.)
> 
[reorganized]
> I see:
> 
>        The  availability  of individual Landlock features is versioned
>        through ABI levels:
> 
>        ┌────┬────────┬────────────────────────────────────────────────┐
>        │ABI │ Kernel │ Newly introduced access rights                 │
>        ├────┼────────┼────────────────────────────────────────────────┤
>        │ 1  │  5.13  │ LANDLOCK_ACCESS_FS_EXECUTE                     │
>        │    │        │ LANDLOCK_ACCESS_FS_WRITE_FILE                  │
>        │    │        │ LANDLOCK_ACCESS_FS_READ_FILE                   │
>        │    │        │ LANDLOCK_ACCESS_FS_READ_DIR                    │
>        │    │        │ LANDLOCK_ACCESS_FS_REMOVE_DIR                  │
>        │    │        │ LANDLOCK_ACCESS_FS_REMOVE_FILE                 │
>        │    │        │ LANDLOCK_ACCESS_FS_MAKE_CHAR                   │
>        │    │        │ LANDLOCK_ACCESS_FS_MAKE_DIR                    │
>        │    │        │ LANDLOCK_ACCESS_FS_MAKE_REG                    │
>        │    │        │ LANDLOCK_ACCESS_FS_MAKE_SOCK                   │
>        │    │        │ LANDLOCK_ACCESS_FS_MAKE_FIFO                   │
>        │    │        │ LANDLOCK_ACCESS_FS_MAKE_BLOCK                  │
>        │    │        │ LANDLOCK_ACCESS_FS_MAKE_SYM                    │
>        ├────┼────────┼────────────────────────────────────────────────┤
>        │ 2  │  5.19  │ LANDLOCK_ACCESS_FS_REFER                       │
>        └────┴────────┴────────────────────────────────────────────────┘
>        To query the running kernel's Landlock ABI level, programs  may
>        pass  the LANDLOCK_CREATE_RULESET_VERSION flag to landlock_cre‐
>        ate_ruleset(2).
[reorganized]
> This sounds weird, but they are right that there seems to be a missing
> blank line.

Yes, they are.

> Could you explain why it's happening?

This is Savannah #49390.

https://savannah.gnu.org/bugs/?49390

It is fixed in groff 1.23.0.  Which, by the way, is at release candidate
3 now.  Final release may be this weekend, depending on Bertrand's
opinion of the changes I've made this week.[1]

> I'd expect the .PP to separate paragraphs with a blank, right?

It does, and it is, but you can't see it because groff 1.22.4 and
earlier table did not move the drawing position below the bottom box
border on nroff devices.

The '.sp 1' workaround (which is synonymous with plain '.sp') can be
removed when you feel groff 1.23.0 has spread sufficiently.

Regards,
Branden

[1] https://git.savannah.gnu.org/cgit/groff.git/log/

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

Re: [PATCH 2/3] landlock.7: Document Landlock ABI v2 (file reparenting; kernel 5.19)

[Alex Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 5204 bytes --]

Hi Branden,

On 2/25/23 02:19, G. Branden Robinson wrote:
> Hi Alex,
> 
> At 2023-02-25T02:10:22+0100, Alex Colomar wrote:
>> On 2/23/23 09:48, Günther Noack wrote:
>>> On Wed, Feb 22, 2023 at 08:36:37AM +0100, Mickaël Salaün wrote:
>>>> On 2023-02-21T21:50:22.000+01:00, Günther Noack wrote:
>>>>> +The availability of individual Landlock features is versioned through
>>>>> +ABI levels:
>>>>> +.TS
>>>>> +box;
>>>>> +ntb| ntb| lbx
>>>>> +nt| nt| lbx.
>>>>> +ABI	Kernel	Newly introduced access rights
>>>>> +_	_	_
>>>>> +1	5.13	LANDLOCK_ACCESS_FS_EXECUTE
>>>>> +\^	\^	LANDLOCK_ACCESS_FS_WRITE_FILE
>>>>> +\^	\^	LANDLOCK_ACCESS_FS_READ_FILE
>>>>> +\^	\^	LANDLOCK_ACCESS_FS_READ_DIR
>>>>> +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_DIR
>>>>> +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_FILE
>>>>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_CHAR
>>>>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_DIR
>>>>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_REG
>>>>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SOCK
>>>>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_FIFO
>>>>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_BLOCK
>>>>> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SYM
>>>>> +_	_	_
>>>>> +2	5.19	LANDLOCK_ACCESS_FS_REFER
>>>>> +.TE
>>>>> +.PP
>>>>
>>>> A line break would be nice here.
>>>
>>> Added. (Used .sp 1 for that, as it is already used in the
>>> mount_namespaces.7, ip.7 and other man pages.)
>>
> [reorganized]
>> I see:
>>
>>         The  availability  of individual Landlock features is versioned
>>         through ABI levels:
>>
>>         ┌────┬────────┬────────────────────────────────────────────────┐
>>         │ABI │ Kernel │ Newly introduced access rights                 │
>>         ├────┼────────┼────────────────────────────────────────────────┤
>>         │ 1  │  5.13  │ LANDLOCK_ACCESS_FS_EXECUTE                     │
>>         │    │        │ LANDLOCK_ACCESS_FS_WRITE_FILE                  │
>>         │    │        │ LANDLOCK_ACCESS_FS_READ_FILE                   │
>>         │    │        │ LANDLOCK_ACCESS_FS_READ_DIR                    │
>>         │    │        │ LANDLOCK_ACCESS_FS_REMOVE_DIR                  │
>>         │    │        │ LANDLOCK_ACCESS_FS_REMOVE_FILE                 │
>>         │    │        │ LANDLOCK_ACCESS_FS_MAKE_CHAR                   │
>>         │    │        │ LANDLOCK_ACCESS_FS_MAKE_DIR                    │
>>         │    │        │ LANDLOCK_ACCESS_FS_MAKE_REG                    │
>>         │    │        │ LANDLOCK_ACCESS_FS_MAKE_SOCK                   │
>>         │    │        │ LANDLOCK_ACCESS_FS_MAKE_FIFO                   │
>>         │    │        │ LANDLOCK_ACCESS_FS_MAKE_BLOCK                  │
>>         │    │        │ LANDLOCK_ACCESS_FS_MAKE_SYM                    │
>>         ├────┼────────┼────────────────────────────────────────────────┤
>>         │ 2  │  5.19  │ LANDLOCK_ACCESS_FS_REFER                       │
>>         └────┴────────┴────────────────────────────────────────────────┘
>>         To query the running kernel's Landlock ABI level, programs  may
>>         pass  the LANDLOCK_CREATE_RULESET_VERSION flag to landlock_cre‐
>>         ate_ruleset(2).
> [reorganized]
>> This sounds weird, but they are right that there seems to be a missing
>> blank line.
> 
> Yes, they are.
> 
>> Could you explain why it's happening?
> 
> This is Savannah #49390.
> 
> https://savannah.gnu.org/bugs/?49390
> 
> It is fixed in groff 1.23.0.  Which, by the way, is at release candidate
> 3 now.

Ahh, I forgot that in my laptop I still have 1.22.4 :)
Good to know that it's already fixed.  Thanks!

>  Final release may be this weekend, depending on Bertrand's
> opinion of the changes I've made this week.[1]

Nice, we'll hopefully have it before I turn 30 (which BTW coincides with 
the deadline for having it in Bookworm, AFAIK :p)

> 
>> I'd expect the .PP to separate paragraphs with a blank, right?
> 
> It does, and it is, but you can't see it because groff 1.22.4 and
> earlier table did not move the drawing position below the bottom box
> border on nroff devices.
> 
> The '.sp 1' workaround (which is synonymous with plain '.sp') can be
> removed when you feel groff 1.23.0 has spread sufficiently.

Since the bug is so minor, I'm in favor of removing it as soon as you 
release.  A little bit of lack-of-blank-line inconvenience is not so bad.

Cheers,

Alex

> 
> Regards,
> Branden
> 
> [1] https://git.savannah.gnu.org/cgit/groff.git/log/

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


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

Re: [PATCH 2/3] landlock.7: Document Landlock ABI v2 (file reparenting; kernel 5.19)

[Günther Noack]

On Sat, Feb 25, 2023 at 02:29:12AM +0100, Alex Colomar wrote:
> On 2/25/23 02:19, G. Branden Robinson wrote:
> > The '.sp 1' workaround (which is synonymous with plain '.sp') can be
> > removed when you feel groff 1.23.0 has spread sufficiently.
> 
> Since the bug is so minor, I'm in favor of removing it as soon as you
> release.  A little bit of lack-of-blank-line inconvenience is not so bad.

OK, thanks for investigating this!

I'll keep it in the man page for now then.

–-Günther

Re: [PATCH 2/3] landlock.7: Document Landlock ABI v2 (file reparenting; kernel 5.19)

[Günther Noack]

Hello Alex!

Thank you for the review! Please see answers inline:

On Sat, Feb 25, 2023 at 12:21:43AM +0100, Alex Colomar wrote:
> On 2/21/23 21:50, Günther Noack wrote:
> Please have a look at man-pages(7):
>    Use semantic newlines
>        [...]
> 
> Here,
> that would mean
> breaking the line right before the opening parenthesis;
> please also apply it to the rest of the patch where appropriate.

Thanks,
I applied this
so that the text is properly formatted now,
according to man-pages(7).

> > +.IP \(bu 3
> 
> We now use \[bu] instead of \(bu (they are equivalent).

Thanks, applied.

> 80 columns is a strong limit.
> Using semantic newlines as suggested above should fix this.

Done.

> > +.TS
> > +box;
> > +ntb| ntb| lbx
> > +nt| nt| lbx.
> > +ABI	Kernel	Newly introduced access rights
> > +_	_	_
> > +1	5.13	LANDLOCK_ACCESS_FS_EXECUTE
> > +\^	\^	LANDLOCK_ACCESS_FS_WRITE_FILE
> 
> What character do you want here?
> If you want ASCII 0x5E,
> then you want to use \[ha].

I don't want it to show a character there.  The intent is to make the
table cells for ABI version and kernel version span across multiple
rows, for the case where a single kernel version introduced multiple
access rights at once.  \^ is a data item in the tbl(1) syntax which
has that special meaning.

https://manpages.debian.org/testing/groff-base/tbl.1.en.html

    A data item consisting only of ‘\^’ indicates that the field
    immediately above spans downward over this row.

> > +\^	\^	LANDLOCK_ACCESS_FS_READ_FILE
> > +\^	\^	LANDLOCK_ACCESS_FS_READ_DIR
> > +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_DIR
> > +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_FILE
> > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_CHAR
> > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_DIR
> > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_REG
> > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SOCK
> > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_FIFO
> > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_BLOCK
> > +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SYM
> > +_	_	_
> > +2	5.19	LANDLOCK_ACCESS_FS_REFER

Thanks for the review!
–Günther

Re: [PATCH 3/3] landlock.7: Document Landlock ABI v3 (file truncation; kernel 6.2)

[Günther Noack]

On Sat, Feb 25, 2023 at 12:31:38AM +0100, Alex Colomar wrote:
> On 2/21/23 21:50, Günther Noack wrote:
> > +Open a file with write access. Note that you might additionally need the
> 
> Again,
> please use semantic newlines.
> 
> Also,
> roff(7) requires two spaces after period in source code,
> to detect sentence endings.
> I say this as a curiosity,
> since due to the requirement of semantic newlines,
> we always write a newline after period.

Applied semantic newlines.  There are no single-space sentence ends left.

> > +Whether an opened file can be truncated with
> > +.BR ftruncate (2)
> > +is determined during
> > +.BR open (2),
> > +in the same way as read and write permissions are checked during
> > +.BR open (2)
> > +using
> > +.B LANDLOCK_ACCESS_FS_READ_FILE
> > +and
> > +.BR LANDLOCK_ACCESS_FS_WRITE_FILE .
> > +This access right is available since the third version of the Landlock ABI.
> 
> Maybe it's simpler to say Landlock ABI v3?  That's usual convention in
> software, so not using formal English for that is not a crime :).

Hm, I took that sentence from the kernel documentation,
and I would like to avoid deviating in minor wording aspects.
https://docs.kernel.org/userspace-api/landlock.html

I would be in favor of "ABI v3" as well though, for brevity.
Mickaël, do you have a strong opinion on this?

Thank you for the review!
–Günther