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

[PATCH v2 1/2] 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 | 102 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 100 insertions(+), 2 deletions(-)

diff --git a/man7/landlock.7 b/man7/landlock.7
index b2bc9e10b..f70a01484 100644
--- a/man7/landlock.7
+++ b/man7/landlock.7
@@ -105,6 +105,56 @@ 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 +232,54 @@ 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
+To determine which Landlock features are available,
+users should query the Landlock ABI version:
+.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
+.sp 1
+.PP
+Users should use the Landlock ABI version rather than the kernel version
+to determine which features are available.
+The mainline kernel versions listed here are only included for orientation.
+Kernels from other sources may contain backported features,
+and their version numbers may not match.
+.PP
+To query the running kernel's Landlock ABI version,
+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 +339,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) {

base-commit: 53a7e5dfc3554a2e8dbdfdc4504e99652e1d6382
-- 
2.39.2

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

[Günther Noack]

https://git.kernel.org/torvalds/c/299e2b1967578b1442128ba8b3e86ed3427d3651
---
 man7/landlock.7 | 82 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 80 insertions(+), 2 deletions(-)

diff --git a/man7/landlock.7 b/man7/landlock.7
index f70a01484..9ddb17ae8 100644
--- a/man7/landlock.7
+++ b/man7/landlock.7
@@ -64,9 +64,38 @@ Execute a file.
 .TP
 .B LANDLOCK_ACCESS_FS_WRITE_FILE
 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,
@@ -231,6 +260,53 @@ 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
@@ -257,6 +333,8 @@ _	_	_
 \^	\^	LANDLOCK_ACCESS_FS_MAKE_SYM
 _	_	_
 2	5.19	LANDLOCK_ACCESS_FS_REFER
+_	_	_
+3	6.2	LANDLOCK_ACCESS_FS_TRUNCATE
 .TE
 .sp 1
 .PP
@@ -302,7 +380,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),
@@ -340,7 +417,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 v2 2/2] landlock.7: Document Landlock ABI v3 (file truncation; kernel 6.2)

[Alejandro Colomar]

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

In the subject it's not so important, but for consistency with the
language used within the pages, I'd ask you to rewrite it as Linux 6.2
(and similarly for patch 1/2).

On 2/28/23 21:52, Günther Noack wrote:
> https://git.kernel.org/torvalds/c/299e2b1967578b1442128ba8b3e86ed3427d3651
> ---
>  man7/landlock.7 | 82 +++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 80 insertions(+), 2 deletions(-)
> 
> diff --git a/man7/landlock.7 b/man7/landlock.7
> index f70a01484..9ddb17ae8 100644
> --- a/man7/landlock.7
> +++ b/man7/landlock.7
> @@ -64,9 +64,38 @@ Execute a file.
>  .TP
>  .B LANDLOCK_ACCESS_FS_WRITE_FILE
>  Open a file with write access.
> +Note that you might additionally need the

"Note that" is usually redundant.
See:
<https://lore.kernel.org/linux-man/20210729223535.qvyomfqvvahzmu5w@localhost.localdomain/>
<https://lore.kernel.org/linux-man/20230105225235.6cjtz6orjzxzvo6v@illithid/>

> +.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,
> @@ -231,6 +260,53 @@ 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" .

Expressions should go in italics.  See man-pages(7):

       Expressions, if not written on a separate indented line, should
       be  specified in italics.  Again, the use of nonbreaking spaces
       may be appropriate if the expression  is  inlined  with  normal
       text.

Cheers,

Alex

> +.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
> @@ -257,6 +333,8 @@ _	_	_
>  \^	\^	LANDLOCK_ACCESS_FS_MAKE_SYM
>  _	_	_
>  2	5.19	LANDLOCK_ACCESS_FS_REFER
> +_	_	_
> +3	6.2	LANDLOCK_ACCESS_FS_TRUNCATE
>  .TE
>  .sp 1
>  .PP
> @@ -302,7 +380,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),
> @@ -340,7 +417,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 v2 1/2] landlock.7: Document Landlock ABI v2 (file reparenting; kernel 5.19)

[Alejandro Colomar]

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

Hi Günther,

On 2/28/23 21:52, 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)
> ---
>  man7/landlock.7 | 102 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 100 insertions(+), 2 deletions(-)
> 
> diff --git a/man7/landlock.7 b/man7/landlock.7
> index b2bc9e10b..f70a01484 100644
> --- a/man7/landlock.7
> +++ b/man7/landlock.7
> @@ -105,6 +105,56 @@ 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

You only need to specify the indentation (the "3") in the first
consecutive .IP.  All others reuse the indentation level until
a .PP appears (or a few other situations that I won't enumerate
for brevity).

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

The * should be in italics, since it's not part of the literal,
but rather a variable part.  I know the pages are not very
consistent in this, but I'd like to make them consistent in the
future.

Cheers,

Alex

> +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 +232,54 @@ 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
> +To determine which Landlock features are available,
> +users should query the Landlock ABI version:
> +.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
> +.sp 1
> +.PP
> +Users should use the Landlock ABI version rather than the kernel version
> +to determine which features are available.
> +The mainline kernel versions listed here are only included for orientation.
> +Kernels from other sources may contain backported features,
> +and their version numbers may not match.
> +.PP
> +To query the running kernel's Landlock ABI version,
> +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 +339,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) {
> 
> base-commit: 53a7e5dfc3554a2e8dbdfdc4504e99652e1d6382

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

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

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

[Günther Noack]

Hello Alex!

On Wed, Mar 01, 2023 at 10:21:13PM +0100, Alejandro Colomar wrote:
> In the subject it's not so important, but for consistency with the
> language used within the pages, I'd ask you to rewrite it as Linux 6.2
> (and similarly for patch 1/2).

Done, sure thing.


> On 2/28/23 21:52, Günther Noack wrote:
> > diff --git a/man7/landlock.7 b/man7/landlock.7
> > index f70a01484..9ddb17ae8 100644
> > --- a/man7/landlock.7
> > +++ b/man7/landlock.7
> > +Note that you might additionally need the
> 
> "Note that" is usually redundant.
> See:
> <https://lore.kernel.org/linux-man/20210729223535.qvyomfqvvahzmu5w@localhost.localdomain/>
> <https://lore.kernel.org/linux-man/20230105225235.6cjtz6orjzxzvo6v@illithid/>

Thank you, that is a good observation, the "Kemper notectomy".
This is very helpful, exactly the kind of review I was hoping for. :)

How about this rewording:

  LANDLOCK_ACCESS_FS_WRITE

    Open a file with write access.

    When opening files for writing, you will often additionally need
    the LANDLOCK_ACCESS_FS_TRUNCATE right.  In many cases, these
    system calls truncate existing files when overwriting them (e.g.
    creat(2)).

This paragraph started with "note that" because it talks about an
unintuitive side-aspect of the LANDLOCK_ACCESS_FS_WRITE_FILE right.

I reworded it to make it more clear why that should be relevant to the
reader (because creat(2) and its open(2) equivalent are used very
commonly).

Does that sound better?

The man page also explains it in more detail in the "Truncating files"
section further below.


> > +system call, this can also be done through
> > +.BR open (2)
> > +with the flags
> > +.BR "O_RDONLY | O_TRUNC" .
> 
> Expressions should go in italics.  See man-pages(7):
> 
>        Expressions, if not written on a separate indented line, should
>        be  specified in italics.  Again, the use of nonbreaking spaces
>        may be appropriate if the expression  is  inlined  with  normal
>        text.

Done.

Thank you for the review!
–Günther

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

[Günther Noack]

Hello Alex!

On Wed, Mar 01, 2023 at 10:25:38PM +0100, Alejandro Colomar wrote:
> On 2/28/23 21:52, Günther Noack wrote:
> > +.IP \[bu] 3
> 
> You only need to specify the indentation (the "3") in the first
> consecutive .IP.  All others reuse the indentation level until
> a .PP appears (or a few other situations that I won't enumerate
> for brevity).

Thanks, I missed that.

> > +When renaming, the
> > +.B LANDLOCK_ACCESS_FS_REMOVE_*
> 
> The * should be in italics, since it's not part of the literal,
> but rather a variable part.  I know the pages are not very
> consistent in this, but I'd like to make them consistent in the
> future.

Done.

Thank you for the review!
–Günther

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

[Alejandro Colomar]

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

On 3/2/23 14:42, Günther Noack wrote:
> Hello Alex!

Hello Günther!

> 
> On Wed, Mar 01, 2023 at 10:21:13PM +0100, Alejandro Colomar wrote:
>> In the subject it's not so important, but for consistency with the
>> language used within the pages, I'd ask you to rewrite it as Linux 6.2
>> (and similarly for patch 1/2).
> 
> Done, sure thing.
> 
> 
>> On 2/28/23 21:52, Günther Noack wrote:
>>> diff --git a/man7/landlock.7 b/man7/landlock.7
>>> index f70a01484..9ddb17ae8 100644
>>> --- a/man7/landlock.7
>>> +++ b/man7/landlock.7
>>> +Note that you might additionally need the
>>
>> "Note that" is usually redundant.
>> See:
>> <https://lore.kernel.org/linux-man/20210729223535.qvyomfqvvahzmu5w@localhost.localdomain/>
>> <https://lore.kernel.org/linux-man/20230105225235.6cjtz6orjzxzvo6v@illithid/>
> 
> Thank you, that is a good observation, the "Kemper notectomy".
> This is very helpful, exactly the kind of review I was hoping for. :)

:)

> 
> How about this rewording:
> 
>   LANDLOCK_ACCESS_FS_WRITE
> 
>     Open a file with write access.
> 
>     When opening files for writing, you will often additionally need
>     the LANDLOCK_ACCESS_FS_TRUNCATE right.  In many cases, these
>     system calls truncate existing files when overwriting them (e.g.
>     creat(2)).

Sounds good.

> 
> This paragraph started with "note that" because it talks about an
> unintuitive side-aspect of the LANDLOCK_ACCESS_FS_WRITE_FILE right.
> 
> I reworded it to make it more clear why that should be relevant to the
> reader (because creat(2) and its open(2) equivalent are used very
> commonly).
> 
> Does that sound better?
> 
> The man page also explains it in more detail in the "Truncating files"
> section further below.
> 
> 
>>> +system call, this can also be done through
>>> +.BR open (2)
>>> +with the flags
>>> +.BR "O_RDONLY | O_TRUNC" .
>>
>> Expressions should go in italics.  See man-pages(7):
>>
>>        Expressions, if not written on a separate indented line, should
>>        be  specified in italics.  Again, the use of nonbreaking spaces
>>        may be appropriate if the expression  is  inlined  with  normal
>>        text.
> 
> Done.
> 
> Thank you for the review!
> –Günther

Cheers,

Alex


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

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