[PATCH v1 0/3] Improve Landlock help

[PATCH v1 0/3] Improve Landlock help

[Mickaël Salaün]

This series improves the documentation as well as the sandboxer sample.

The documentation style changes were previously discussed:
https://lore.kernel.org/r/2f9c6131-3140-9c47-cf95-f7fa3cf759ee@digikod.net

This conflict with the ongoing truncate and network patch series but the
resolution should be simple, see my "next" branch merge:
https://git.kernel.org/mic/c/3d6723ed39cf13e6da69eee765d01c3bc1d315a5

Mickaël Salaün (3):
  samples/landlock: Print hints about ABI versions
  landlock: Slightly improve documentation and fix spelling
  landlock: Fix documentation style

 Documentation/security/landlock.rst      |  8 ++---
 Documentation/userspace-api/landlock.rst | 33 +++++++++----------
 include/uapi/linux/landlock.h            | 10 +++---
 samples/landlock/sandboxer.c             | 37 +++++++++++++++++-----
 security/landlock/fs.c                   |  2 +-
 security/landlock/syscalls.c             | 40 ++++++++++++------------
 6 files changed, 76 insertions(+), 54 deletions(-)


base-commit: 521a547ced6477c54b4b0cc206000406c221b4d6
-- 
2.37.2

[PATCH v1 1/3] samples/landlock: Print hints about ABI versions

[Mickaël Salaün]

Extend the help with the latest Landlock ABI version supported by the
sandboxer.

Inform users about the sandboxer or the kernel not being up-to-date.

Make the version check code easier to update and harder to misuse.

Cc: Günther Noack <gnoack3000@gmail.com>
Cc: Paul Moore <paul@paul-moore.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20220923154207.3311629-2-mic@digikod.net
---
 samples/landlock/sandboxer.c | 37 ++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index 3e404e51ec64..f29bb3c72230 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -162,11 +162,10 @@ static int populate_ruleset(const char *const env_var, const int ruleset_fd,
 	LANDLOCK_ACCESS_FS_MAKE_SYM | \
 	LANDLOCK_ACCESS_FS_REFER)
 
-#define ACCESS_ABI_2 ( \
-	LANDLOCK_ACCESS_FS_REFER)
-
 /* clang-format on */
 
+#define LANDLOCK_ABI_LAST 2
+
 int main(const int argc, char *const argv[], char *const *const envp)
 {
 	const char *cmd_path;
@@ -196,8 +195,12 @@ int main(const int argc, char *const argv[], char *const *const envp)
 			"\nexample:\n"
 			"%s=\"/bin:/lib:/usr:/proc:/etc:/dev/urandom\" "
 			"%s=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" "
-			"%s bash -i\n",
+			"%s bash -i\n\n",
 			ENV_FS_RO_NAME, ENV_FS_RW_NAME, argv[0]);
+		fprintf(stderr,
+			"This sandboxer can use Landlock features "
+			"up to ABI version %d.\n",
+			LANDLOCK_ABI_LAST);
 		return 1;
 	}
 
@@ -225,12 +228,30 @@ int main(const int argc, char *const argv[], char *const *const envp)
 		}
 		return 1;
 	}
+
 	/* Best-effort security. */
-	if (abi < 2) {
-		ruleset_attr.handled_access_fs &= ~ACCESS_ABI_2;
-		access_fs_ro &= ~ACCESS_ABI_2;
-		access_fs_rw &= ~ACCESS_ABI_2;
+	switch (abi) {
+	case 1:
+		/* Removes LANDLOCK_ACCESS_FS_REFER for ABI < 2 */
+		ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_REFER;
+
+		fprintf(stderr,
+			"Hint: You should update the running kernel "
+			"to leverage Landlock features "
+			"provided by ABI version %d (instead of %d).\n",
+			LANDLOCK_ABI_LAST, abi);
+		__attribute__((fallthrough));
+	case LANDLOCK_ABI_LAST:
+		break;
+	default:
+		fprintf(stderr,
+			"Hint: You should update this sandboxer "
+			"to leverage Landlock features "
+			"provided by ABI version %d (instead of %d).\n",
+			abi, LANDLOCK_ABI_LAST);
 	}
+	access_fs_ro &= ruleset_attr.handled_access_fs;
+	access_fs_rw &= ruleset_attr.handled_access_fs;
 
 	ruleset_fd =
 		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
-- 
2.37.2

Re: [PATCH v1 1/3] samples/landlock: Print hints about ABI versions

[Günther Noack]

Reviewed-by: Günther Noack <gnoack3000@gmail.com>

This patch is a strict improvement over what the sample code was
before, so that's fine with me review wise.

I still think it would be good to point out more explicitly that the
"refer" right needs a different fallback strategy for the best effort
mode than the other rights will do in the future, as discussed in [1].

In many "best effort" scenarios that people need for their code, the
part that is actually fixed are the access rights that their code
declares that it needs. So if they actually need the "refer" right for
their programs to work, they cannot use Landlock on kernels that only
support Landlock ABI v1, because under ABI v1 they will never be able
to hardlink or rename between directories when Landlock is enabled.

The way that the sandboxer example is dealing with it, it just gives
the user a smaller set of access rights than they requested if the
kernel just supports ABI v1. It's somewhat reasonable for the
sandboxer tool to do because it doesn't give hard guarantees in its
command line interface, but it might not be negotiable in more
practical examples. :)

[1] https://docs.google.com/document/d/1SkFpl_Xxyl4E6G2uYIlzL0gY2PFo-Nl8ikblLvnpvlU/edit

—Günther

On Fri, Sep 23, 2022 at 05:42:05PM +0200, Mickaël Salaün wrote:
> Extend the help with the latest Landlock ABI version supported by the
> sandboxer.
> 
> Inform users about the sandboxer or the kernel not being up-to-date.
> 
> Make the version check code easier to update and harder to misuse.
> 
> Cc: Günther Noack <gnoack3000@gmail.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Link: https://lore.kernel.org/r/20220923154207.3311629-2-mic@digikod.net
> ---
>  samples/landlock/sandboxer.c | 37 ++++++++++++++++++++++++++++--------
>  1 file changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
> index 3e404e51ec64..f29bb3c72230 100644
> --- a/samples/landlock/sandboxer.c
> +++ b/samples/landlock/sandboxer.c
> @@ -162,11 +162,10 @@ static int populate_ruleset(const char *const env_var, const int ruleset_fd,
>  	LANDLOCK_ACCESS_FS_MAKE_SYM | \
>  	LANDLOCK_ACCESS_FS_REFER)
>  
> -#define ACCESS_ABI_2 ( \
> -	LANDLOCK_ACCESS_FS_REFER)
> -
>  /* clang-format on */
>  
> +#define LANDLOCK_ABI_LAST 2
> +
>  int main(const int argc, char *const argv[], char *const *const envp)
>  {
>  	const char *cmd_path;
> @@ -196,8 +195,12 @@ int main(const int argc, char *const argv[], char *const *const envp)
>  			"\nexample:\n"
>  			"%s=\"/bin:/lib:/usr:/proc:/etc:/dev/urandom\" "
>  			"%s=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" "
> -			"%s bash -i\n",
> +			"%s bash -i\n\n",
>  			ENV_FS_RO_NAME, ENV_FS_RW_NAME, argv[0]);
> +		fprintf(stderr,
> +			"This sandboxer can use Landlock features "
> +			"up to ABI version %d.\n",
> +			LANDLOCK_ABI_LAST);
>  		return 1;
>  	}
>  
> @@ -225,12 +228,30 @@ int main(const int argc, char *const argv[], char *const *const envp)
>  		}
>  		return 1;
>  	}
> +
>  	/* Best-effort security. */
> -	if (abi < 2) {
> -		ruleset_attr.handled_access_fs &= ~ACCESS_ABI_2;
> -		access_fs_ro &= ~ACCESS_ABI_2;
> -		access_fs_rw &= ~ACCESS_ABI_2;
> +	switch (abi) {
> +	case 1:
> +		/* Removes LANDLOCK_ACCESS_FS_REFER for ABI < 2 */
> +		ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_REFER;
> +
> +		fprintf(stderr,
> +			"Hint: You should update the running kernel "
> +			"to leverage Landlock features "
> +			"provided by ABI version %d (instead of %d).\n",
> +			LANDLOCK_ABI_LAST, abi);
> +		__attribute__((fallthrough));
> +	case LANDLOCK_ABI_LAST:
> +		break;
> +	default:
> +		fprintf(stderr,
> +			"Hint: You should update this sandboxer "
> +			"to leverage Landlock features "
> +			"provided by ABI version %d (instead of %d).\n",
> +			abi, LANDLOCK_ABI_LAST);
>  	}
> +	access_fs_ro &= ruleset_attr.handled_access_fs;
> +	access_fs_rw &= ruleset_attr.handled_access_fs;
>  
>  	ruleset_fd =
>  		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
> -- 
> 2.37.2
> 

-- 

Re: [PATCH v1 1/3] samples/landlock: Print hints about ABI versions

[Mickaël Salaün]

On 23/09/2022 23:02, Günther Noack wrote:
> Reviewed-by: Günther Noack <gnoack3000@gmail.com>
> 
> This patch is a strict improvement over what the sample code was
> before, so that's fine with me review wise.
> 
> I still think it would be good to point out more explicitly that the
> "refer" right needs a different fallback strategy for the best effort
> mode than the other rights will do in the future, as discussed in [1].
> 
> In many "best effort" scenarios that people need for their code, the
> part that is actually fixed are the access rights that their code
> declares that it needs. So if they actually need the "refer" right for
> their programs to work, they cannot use Landlock on kernels that only
> support Landlock ABI v1, because under ABI v1 they will never be able
> to hardlink or rename between directories when Landlock is enabled.
> 
> The way that the sandboxer example is dealing with it, it just gives
> the user a smaller set of access rights than they requested if the
> kernel just supports ABI v1. It's somewhat reasonable for the
> sandboxer tool to do because it doesn't give hard guarantees in its
> command line interface, but it might not be negotiable in more
> practical examples. :)
> 
> [1] https://docs.google.com/document/d/1SkFpl_Xxyl4E6G2uYIlzL0gY2PFo-Nl8ikblLvnpvlU/edit

I agree. The sandboxer is a sample, and such sandboxer is not the best 
place to configure Landlock according to the app semantic. At the end it 
should be done in the app itself.

I would like this sample to be as simple as possible but still useful. 
To properly handle all "refer" use cases, it would require a dedicated 
configuration (e.g. LL_FS_REFER), which will make it more difficult to 
understand, and this approach will not scale with future (FS) access rights.

We can maybe add a comment in this sample to explain that. Your 
explanation looks like a good start. If you agree, could you send a 
patch to add such comment (on top of this series)?

> 
> —Günther
> 
> On Fri, Sep 23, 2022 at 05:42:05PM +0200, Mickaël Salaün wrote:
>> Extend the help with the latest Landlock ABI version supported by the
>> sandboxer.
>>
>> Inform users about the sandboxer or the kernel not being up-to-date.
>>
>> Make the version check code easier to update and harder to misuse.
>>
>> Cc: Günther Noack <gnoack3000@gmail.com>
>> Cc: Paul Moore <paul@paul-moore.com>
>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>> Link: https://lore.kernel.org/r/20220923154207.3311629-2-mic@digikod.net
>> ---
>>   samples/landlock/sandboxer.c | 37 ++++++++++++++++++++++++++++--------
>>   1 file changed, 29 insertions(+), 8 deletions(-)
>>
>> diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
>> index 3e404e51ec64..f29bb3c72230 100644
>> --- a/samples/landlock/sandboxer.c
>> +++ b/samples/landlock/sandboxer.c
>> @@ -162,11 +162,10 @@ static int populate_ruleset(const char *const env_var, const int ruleset_fd,
>>   	LANDLOCK_ACCESS_FS_MAKE_SYM | \
>>   	LANDLOCK_ACCESS_FS_REFER)
>>   
>> -#define ACCESS_ABI_2 ( \
>> -	LANDLOCK_ACCESS_FS_REFER)
>> -
>>   /* clang-format on */
>>   
>> +#define LANDLOCK_ABI_LAST 2
>> +
>>   int main(const int argc, char *const argv[], char *const *const envp)
>>   {
>>   	const char *cmd_path;
>> @@ -196,8 +195,12 @@ int main(const int argc, char *const argv[], char *const *const envp)
>>   			"\nexample:\n"
>>   			"%s=\"/bin:/lib:/usr:/proc:/etc:/dev/urandom\" "
>>   			"%s=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" "
>> -			"%s bash -i\n",
>> +			"%s bash -i\n\n",
>>   			ENV_FS_RO_NAME, ENV_FS_RW_NAME, argv[0]);
>> +		fprintf(stderr,
>> +			"This sandboxer can use Landlock features "
>> +			"up to ABI version %d.\n",
>> +			LANDLOCK_ABI_LAST);
>>   		return 1;
>>   	}
>>   
>> @@ -225,12 +228,30 @@ int main(const int argc, char *const argv[], char *const *const envp)
>>   		}
>>   		return 1;
>>   	}
>> +
>>   	/* Best-effort security. */
>> -	if (abi < 2) {
>> -		ruleset_attr.handled_access_fs &= ~ACCESS_ABI_2;
>> -		access_fs_ro &= ~ACCESS_ABI_2;
>> -		access_fs_rw &= ~ACCESS_ABI_2;
>> +	switch (abi) {
>> +	case 1:
>> +		/* Removes LANDLOCK_ACCESS_FS_REFER for ABI < 2 */
>> +		ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_REFER;
>> +
>> +		fprintf(stderr,
>> +			"Hint: You should update the running kernel "
>> +			"to leverage Landlock features "
>> +			"provided by ABI version %d (instead of %d).\n",
>> +			LANDLOCK_ABI_LAST, abi);
>> +		__attribute__((fallthrough));
>> +	case LANDLOCK_ABI_LAST:
>> +		break;
>> +	default:
>> +		fprintf(stderr,
>> +			"Hint: You should update this sandboxer "
>> +			"to leverage Landlock features "
>> +			"provided by ABI version %d (instead of %d).\n",
>> +			abi, LANDLOCK_ABI_LAST);
>>   	}
>> +	access_fs_ro &= ruleset_attr.handled_access_fs;
>> +	access_fs_rw &= ruleset_attr.handled_access_fs;
>>   
>>   	ruleset_fd =
>>   		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
>> -- 
>> 2.37.2
>>
> 

[PATCH v1 2/3] landlock: Slightly improve documentation and fix spelling

[Mickaël Salaün]

Now that we have more than one ABI version, make limitation explanation
more consistent by replacing "ABI 1" with "ABI < 2".  This also
indicates which ABIs support such past limitation.

Improve documentation consistency by not using contractions.

Fix spelling in fs.c .

Cc: Günther Noack <gnoack3000@gmail.com>
Cc: Paul Moore <paul@paul-moore.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20220923154207.3311629-3-mic@digikod.net
---
 Documentation/security/landlock.rst      |  4 ++--
 Documentation/userspace-api/landlock.rst | 10 +++++-----
 security/landlock/fs.c                   |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/security/landlock.rst b/Documentation/security/landlock.rst
index 5c77730b4479..cc9617f3175b 100644
--- a/Documentation/security/landlock.rst
+++ b/Documentation/security/landlock.rst
@@ -7,7 +7,7 @@ Landlock LSM: kernel documentation
 ==================================
 
 :Author: Mickaël Salaün
-:Date: May 2022
+:Date: September 2022
 
 Landlock's goal is to create scoped access-control (i.e. sandboxing).  To
 harden a whole system, this feature should be available to any process,
@@ -49,7 +49,7 @@ Filesystem access rights
 ------------------------
 
 All access rights are tied to an inode and what can be accessed through it.
-Reading the content of a directory doesn't imply to be allowed to read the
+Reading the content of a directory does not imply to be allowed to read the
 content of a listed inode.  Indeed, a file name is local to its parent
 directory, and an inode can be referenced by multiple file names thanks to
 (hard) links.  Being able to unlink a file only has a direct impact on the
diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index b8ea59493964..83bae71bf042 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -8,7 +8,7 @@ Landlock: unprivileged access control
 =====================================
 
 :Author: Mickaël Salaün
-:Date: May 2022
+:Date: September 2022
 
 The goal of Landlock is to enable to restrict ambient rights (e.g. global
 filesystem access) for a set of processes.  Because Landlock is a stackable
@@ -170,7 +170,7 @@ It is recommended setting access rights to file hierarchy leaves as much as
 possible.  For instance, it is better to be able to have ``~/doc/`` as a
 read-only hierarchy and ``~/tmp/`` as a read-write hierarchy, compared to
 ``~/`` as a read-only hierarchy and ``~/tmp/`` as a read-write hierarchy.
-Following this good practice leads to self-sufficient hierarchies that don't
+Following this good practice leads to self-sufficient hierarchies that do not
 depend on their location (i.e. parent directories).  This is particularly
 relevant when we want to allow linking or renaming.  Indeed, having consistent
 access rights per directory enables to change the location of such directory
@@ -380,8 +380,8 @@ by the Documentation/admin-guide/cgroup-v1/memory.rst.
 Previous limitations
 ====================
 
-File renaming and linking (ABI 1)
----------------------------------
+File renaming and linking (ABI < 2)
+-----------------------------------
 
 Because Landlock targets unprivileged access controls, it needs to properly
 handle composition of rules.  Such property also implies rules nesting.
@@ -410,7 +410,7 @@ contains `CONFIG_LSM=landlock,[...]` with `[...]`  as the list of other
 potentially useful security modules for the running system (see the
 `CONFIG_LSM` help).
 
-If the running kernel doesn't have `landlock` in `CONFIG_LSM`, then we can
+If the running kernel does not have `landlock` in `CONFIG_LSM`, then we can
 still enable it by adding ``lsm=landlock,[...]`` to
 Documentation/admin-guide/kernel-parameters.rst thanks to the bootloader
 configuration.
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index a9dbd99d9ee7..64ed7665455f 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -712,7 +712,7 @@ static inline access_mask_t maybe_remove(const struct dentry *const dentry)
  * allowed accesses in @layer_masks_dom.
  *
  * This is similar to check_access_path_dual() but much simpler because it only
- * handles walking on the same mount point and only check one set of accesses.
+ * handles walking on the same mount point and only checks one set of accesses.
  *
  * Returns:
  * - true if all the domain access rights are allowed for @dir;
-- 
2.37.2

Re: [PATCH v1 2/3] landlock: Slightly improve documentation and fix spelling

[Günther Noack]

Reviewed-by: Günther Noack <gnoack3000@gmail.com>

On Fri, Sep 23, 2022 at 05:42:06PM +0200, Mickaël Salaün wrote:
> Now that we have more than one ABI version, make limitation explanation
> more consistent by replacing "ABI 1" with "ABI < 2".  This also
> indicates which ABIs support such past limitation.
> 
> Improve documentation consistency by not using contractions.
> 
> Fix spelling in fs.c .
> 
> Cc: Günther Noack <gnoack3000@gmail.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Link: https://lore.kernel.org/r/20220923154207.3311629-3-mic@digikod.net
> ---
>  Documentation/security/landlock.rst      |  4 ++--
>  Documentation/userspace-api/landlock.rst | 10 +++++-----
>  security/landlock/fs.c                   |  2 +-
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/security/landlock.rst b/Documentation/security/landlock.rst
> index 5c77730b4479..cc9617f3175b 100644
> --- a/Documentation/security/landlock.rst
> +++ b/Documentation/security/landlock.rst
> @@ -7,7 +7,7 @@ Landlock LSM: kernel documentation
>  ==================================
>  
>  :Author: Mickaël Salaün
> -:Date: May 2022
> +:Date: September 2022
>  
>  Landlock's goal is to create scoped access-control (i.e. sandboxing).  To
>  harden a whole system, this feature should be available to any process,
> @@ -49,7 +49,7 @@ Filesystem access rights
>  ------------------------
>  
>  All access rights are tied to an inode and what can be accessed through it.
> -Reading the content of a directory doesn't imply to be allowed to read the
> +Reading the content of a directory does not imply to be allowed to read the
>  content of a listed inode.  Indeed, a file name is local to its parent
>  directory, and an inode can be referenced by multiple file names thanks to
>  (hard) links.  Being able to unlink a file only has a direct impact on the
> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> index b8ea59493964..83bae71bf042 100644
> --- a/Documentation/userspace-api/landlock.rst
> +++ b/Documentation/userspace-api/landlock.rst
> @@ -8,7 +8,7 @@ Landlock: unprivileged access control
>  =====================================
>  
>  :Author: Mickaël Salaün
> -:Date: May 2022
> +:Date: September 2022
>  
>  The goal of Landlock is to enable to restrict ambient rights (e.g. global
>  filesystem access) for a set of processes.  Because Landlock is a stackable
> @@ -170,7 +170,7 @@ It is recommended setting access rights to file hierarchy leaves as much as
>  possible.  For instance, it is better to be able to have ``~/doc/`` as a
>  read-only hierarchy and ``~/tmp/`` as a read-write hierarchy, compared to
>  ``~/`` as a read-only hierarchy and ``~/tmp/`` as a read-write hierarchy.
> -Following this good practice leads to self-sufficient hierarchies that don't
> +Following this good practice leads to self-sufficient hierarchies that do not
>  depend on their location (i.e. parent directories).  This is particularly
>  relevant when we want to allow linking or renaming.  Indeed, having consistent
>  access rights per directory enables to change the location of such directory
> @@ -380,8 +380,8 @@ by the Documentation/admin-guide/cgroup-v1/memory.rst.
>  Previous limitations
>  ====================
>  
> -File renaming and linking (ABI 1)
> ----------------------------------
> +File renaming and linking (ABI < 2)
> +-----------------------------------
>  
>  Because Landlock targets unprivileged access controls, it needs to properly
>  handle composition of rules.  Such property also implies rules nesting.
> @@ -410,7 +410,7 @@ contains `CONFIG_LSM=landlock,[...]` with `[...]`  as the list of other
>  potentially useful security modules for the running system (see the
>  `CONFIG_LSM` help).
>  
> -If the running kernel doesn't have `landlock` in `CONFIG_LSM`, then we can
> +If the running kernel does not have `landlock` in `CONFIG_LSM`, then we can
>  still enable it by adding ``lsm=landlock,[...]`` to
>  Documentation/admin-guide/kernel-parameters.rst thanks to the bootloader
>  configuration.
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index a9dbd99d9ee7..64ed7665455f 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -712,7 +712,7 @@ static inline access_mask_t maybe_remove(const struct dentry *const dentry)
>   * allowed accesses in @layer_masks_dom.
>   *
>   * This is similar to check_access_path_dual() but much simpler because it only
> - * handles walking on the same mount point and only check one set of accesses.
> + * handles walking on the same mount point and only checks one set of accesses.
>   *
>   * Returns:
>   * - true if all the domain access rights are allowed for @dir;
> -- 
> 2.37.2
> 

-- 

Re: [PATCH v1 2/3] landlock: Slightly improve documentation and fix spelling

[Bagas Sanjaya]

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

On Fri, Sep 23, 2022 at 05:42:06PM +0200, Mickaël Salaün wrote:
> Now that we have more than one ABI version, make limitation explanation
> more consistent by replacing "ABI 1" with "ABI < 2".  This also
> indicates which ABIs support such past limitation.
> 

Hi,

I think grammar and reference link on userspace documentation can be
improved, like:

---- >8 ----

diff --git a/Documentation/admin-guide/cgroup-v1/memory.rst b/Documentation/admin-guide/cgroup-v1/memory.rst
index 2cc502a75ef640..c49454900edb12 100644
--- a/Documentation/admin-guide/cgroup-v1/memory.rst
+++ b/Documentation/admin-guide/cgroup-v1/memory.rst
@@ -1,3 +1,5 @@
+.. _cgroup-v1-memory:
+
 ==========================
 Memory Resource Controller
 ==========================
diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
index 4c76fda076454c..dbeddb6851e6c9 100644
--- a/Documentation/filesystems/overlayfs.rst
+++ b/Documentation/filesystems/overlayfs.rst
@@ -3,6 +3,8 @@
 Written by: Neil Brown
 Please see MAINTAINERS file for where to send questions.
 
+.. _overlayfs:
+
 Overlay Filesystem
 ==================
 
diff --git a/Documentation/filesystems/sharedsubtree.rst b/Documentation/filesystems/sharedsubtree.rst
index d83395354250d9..cbc20658bd0c07 100644
--- a/Documentation/filesystems/sharedsubtree.rst
+++ b/Documentation/filesystems/sharedsubtree.rst
@@ -1,5 +1,7 @@
 .. SPDX-License-Identifier: GPL-2.0
 
+.. _shared-subtrees:
+
 ===============
 Shared Subtrees
 ===============
diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index 5dbd577b5f58b7..13a8db59b0c0e5 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -14,16 +14,18 @@ The goal of Landlock is to enable to restrict ambient rights (e.g. global
 filesystem access) for a set of processes.  Because Landlock is a stackable
 LSM, it makes possible to create safe security sandboxes as new security layers
 in addition to the existing system-wide access-controls. This kind of sandbox
-is expected to help mitigate the security impact of bugs or
-unexpected/malicious behaviors in user space applications.  Landlock empowers
-any process, including unprivileged ones, to securely restrict themselves.
+can help mitigating the security impact of bugs or unexpected/malicious
+behaviors in user space applications.  Landlock empowers any process, including
+unprivileged ones, to securely restrict themselves.
 
-We can quickly make sure that Landlock is enabled in the running system by
-looking for ``landlock: Up and running`` in kernel logs (as root): ``dmesg | 
-grep landlock || journalctl -kg landlock`` .  Developers can also easily check
-for Landlock support with a :ref:`related system call <landlock_abi_versions>`.
-If Landlock is not currently supported, we need to :ref:`configure the kernel
-appropriately <kernel_support>`.
+To check whether Landlock has been successfully enabled, look for
+``landlock: Up and running`` in kernel logs, which can be seen with (as root)::
+
+  dmesg | grep landlock || journalctl -kg landlock
+  
+Developers can also easily check for Landlock support with a
+:ref:`related system call <landlock_abi_versions>`. If Landlock is not
+currently supported, :ref:`the kernel needs to be configured <kernel_support>`.
 
 Landlock rules
 ==============
@@ -36,12 +38,12 @@ the thread enforcing it, and its future children.
 Defining and enforcing a security policy
 ----------------------------------------
 
-We first need to define the ruleset that will contain our rules.  For this
-example, the ruleset will contain rules that only allow read actions, but write
-actions will be denied.  The ruleset then needs to handle both of these kind of
+First, the ruleset that will contain rules needs to be defined. For this
+example, the ruleset will contain rules that only allow read actions (write
+actions will be denied). The ruleset then needs to handle both of these kind of
 actions.  This is required for backward and forward compatibility (i.e. the
 kernel and user space may not know each other's supported restrictions), hence
-the need to be explicit about the denied-by-default access rights.
+it is required to be explicit about the denied-by-default access rights.
 
 .. code-block:: c
 
@@ -63,14 +65,14 @@ the need to be explicit about the denied-by-default access rights.
             LANDLOCK_ACCESS_FS_REFER,
     };
 
-Because we may not know on which kernel version an application will be
-executed, it is safer to follow a best-effort security approach.  Indeed, we
-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.
+Because it is unknown which kernel version an application will be executed,
+it is safer to follow a best-effort security approach. Indeed, users need to
+be protected as much as possible whatever the kernel they are
+using.  To avoid atomic enforcement situations (i.e. either all security
+features or none), a dedicated Landlock command to get the current version
+of the Landlock ABI can be used and adapt the handled accesses. Let's check if
+``LANDLOCK_ACCESS_FS_REFER`` access right (which is available since second
+ABI version) can be removed:
 
 .. code-block:: c
 
@@ -81,7 +83,7 @@ starting with the second version of the ABI.
         ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_REFER;
     }
 
-This enables to create an inclusive ruleset that will contain our rules.
+This enables to create an inclusive ruleset that will contain the rules.
 
 .. code-block:: c
 
@@ -93,12 +95,11 @@ This enables to create an inclusive ruleset that will contain our rules.
         return 1;
     }
 
-We can now add a new rule to this ruleset thanks to the returned file
+Now a new rule to this ruleset can be added using the returned file
 descriptor referring to this ruleset.  The rule will only allow reading the
-file hierarchy ``/usr``.  Without another rule, write actions would then be
-denied by the ruleset.  To add ``/usr`` to the ruleset, we open it with the
-``O_PATH`` flag and fill the &struct landlock_path_beneath_attr with this file
-descriptor.
+file hierarchy ``/usr``. To add ``/usr`` to the ruleset, open the ruleset
+with the ``O_PATH`` flag and fill the struct landlock_path_beneath_attr with
+the descriptor:
 
 .. code-block:: c
 
@@ -125,15 +126,15 @@ descriptor.
         return 1;
     }
 
-It may also be required to create rules following the same logic as explained
-for the ruleset creation, by filtering access rights according to the Landlock
-ABI version.  In this example, this is not required because
-``LANDLOCK_ACCESS_FS_REFER`` is not allowed by any rule.
+It may also be required to add rules following the same logic as explained
+above, to filter access rights according to the Landlock ABI version.
+In this example, this is not required because ``LANDLOCK_ACCESS_FS_REFER``
+is not allowed by any rule.
 
-We now have a ruleset with one rule allowing read access to ``/usr`` while
+Now there is a ruleset with one rule allowing read access to ``/usr`` while
 denying all other handled accesses for the filesystem.  The next step is to
-restrict the current thread from gaining more privileges (e.g. thanks to a SUID
-binary).
+restrict the current thread from gaining more privileges (for example due
+to be run by setuid binary):
 
 .. code-block:: c
 
@@ -158,7 +159,7 @@ If the ``landlock_restrict_self`` system call succeeds, the current thread is
 now restricted and this policy will be enforced on all its subsequently created
 children as well.  Once a thread is landlocked, there is no way to remove its
 security policy; only adding more restrictions is allowed.  These threads are
-now in a new Landlock domain, merge of their parent one (if any) with the new
+now in a new Landlock domain, merging their parent one (if any) with the new
 ruleset.
 
 Full working code can be found in `samples/landlock/sandboxer.c`_.
@@ -166,24 +167,24 @@ Full working code can be found in `samples/landlock/sandboxer.c`_.
 Good practices
 --------------
 
-It is recommended setting access rights to file hierarchy leaves as much as
-possible.  For instance, it is better to be able to have ``~/doc/`` as a
-read-only hierarchy and ``~/tmp/`` as a read-write hierarchy, compared to
-``~/`` as a read-only hierarchy and ``~/tmp/`` as a read-write hierarchy.
-Following this good practice leads to self-sufficient hierarchies that do not
-depend on their location (i.e. parent directories).  This is particularly
-relevant when we want to allow linking or renaming.  Indeed, having consistent
-access rights per directory enables to change the location of such directory
-without relying on the destination directory access rights (except those that
-are required for this operation, see ``LANDLOCK_ACCESS_FS_REFER``
-documentation).
+It is recommended to set access rights to file hierarchy leaves as much as
+possible.  For instance, it is better to have a read-only ``~/doc/`` and
+read-write ``~/tmp/`` hierarchies, compared to read-only ``~/`` and
+read-write ``~/tmp/`` hierarchies. Following it leads to self-sufficient
+hierarchies that do not depend on their location (i.e. parent directories).
+This is particularly relevant when allowing linking or renaming is needed.
+Indeed, having consistent access rights per directory enables to change the
+location of such directory without relying on the destination directory access
+rights (except those that are required for this operation, see
+``LANDLOCK_ACCESS_FS_REFER`` documentation for example).
+
 Having self-sufficient hierarchies also helps to tighten the required access
 rights to the minimal set of data.  This also helps avoid sinkhole directories,
-i.e.  directories where data can be linked to but not linked from.  However,
-this depends on data organization, which might not be controlled by developers.
-In this case, granting read-write access to ``~/tmp/``, instead of write-only
-access, would potentially allow to move ``~/tmp/`` to a non-readable directory
-and still keep the ability to list the content of ``~/tmp/``.
+i.e.  directories where data can be linked to but not from.  However,
+this depends on data organization and layout, which may be outside developer
+control. In this case, granting read-write access instead of write-only to
+``~/tmp/`` would potentially allow to move ``~/tmp/`` to a non-readable
+directory and still keep the ability to list the content of ``~/tmp/``.
 
 Layers of file path access rights
 ---------------------------------
@@ -194,7 +195,7 @@ the potentially other rulesets already restricting this thread.  A sandboxed
 thread can then safely add more constraints to itself with a new enforced
 ruleset.
 
-One policy layer grants access to a file path if at least one of its rules
+A policy layer grants access to a file path if at least one of its rules
 encountered on the path grants the access.  A sandboxed thread can only access
 a file path if all its enforced policy layers grant the access as well as all
 the other system access controls (e.g. filesystem DAC, other LSM policies,
@@ -203,42 +204,40 @@ etc.).
 Bind mounts and OverlayFS
 -------------------------
 
-Landlock enables to restrict access to file hierarchies, which means that these
-access rights can be propagated with bind mounts (cf.
-Documentation/filesystems/sharedsubtree.rst) but not with
-Documentation/filesystems/overlayfs.rst.
+Landlock can restrict access to file hierarchies, which means that these
+access rights can be propagated with bind mounts (see
+:ref:`shared-subtrees`) but not with :ref:`OverlayFS <overlayfs>`.
 
 A bind mount mirrors a source file hierarchy to a destination.  The destination
 hierarchy is then composed of the exact same files, on which Landlock rules can
-be tied, either via the source or the destination path.  These rules restrict
-access when they are encountered on a path, which means that they can restrict
-access to multiple file hierarchies at the same time, whether these hierarchies
+be applied, either via the source or the destination path.  These rules
+restrict access when they are encountered on a path, which means that access
+to multiple file hierarchies can be restricted, whether these hierarchies
 are the result of bind mounts or not.
 
-An OverlayFS mount point consists of upper and lower layers.  These layers are
-combined in a merge directory, result of the mount point.  This merge hierarchy
-may include files from the upper and lower layers, but modifications performed
-on the merge hierarchy only reflects on the upper layer.  From a Landlock
-policy point of view, each OverlayFS layers and merge hierarchies are
-standalone and contains their own set of files and directories, which is
-different from bind mounts.  A policy restricting an OverlayFS layer will not
-restrict the resulted merged hierarchy, and vice versa.  Landlock users should
-then only think about file hierarchies they want to allow access to, regardless
-of the underlying filesystem.
+On the other hand, an OverlayFS mount point consists of upper and lower layers.
+These layers are combined in a merge directory; result of the mount point.
+This merge hierarchy may include files from the upper and lower layers, but
+modifications performed on the merge hierarchy only reflects on the upper
+layer. From a Landlock policy point of view, each OverlayFS layers and merge
+hierarchies are standalone and contains their own set of files and directories,
+which are different from bind mounts.  A policy restricting an OverlayFS layer
+will not restrict the resulted merged hierarchy, and vice versa. Landlock users
+should then only think about file hierarchies they want to allow access to,
+regardless of the underlying filesystem.
 
 Inheritance
 -----------
 
 Every new thread resulting from a :manpage:`clone(2)` inherits Landlock domain
-restrictions from its parent.  This is similar to the seccomp inheritance (cf.
-Documentation/userspace-api/seccomp_filter.rst) or any other LSM dealing with
-task's :manpage:`credentials(7)`.  For instance, one process's thread may apply
-Landlock rules to itself, but they will not be automatically applied to other
-sibling threads (unlike POSIX thread credential changes, cf.
-:manpage:`nptl(7)`).
+restrictions from its parent.  This is similar to the :ref:`seccomp-bpf`) or
+any other LSM dealing with task :manpage:`credentials(7)`. For instance, a
+process thread may apply Landlock rules to itself, but they will not be
+automatically applied to other sibling threads (unlike POSIX thread credential
+changes, see :manpage:`nptl(7)`).
 
-When a thread sandboxes itself, we have the guarantee that the related security
-policy will stay enforced on all this thread's descendants.  This allows
+When a thread sandboxes itself, there is a guarantee that the related security
+policy will stay enforced on all the thread descendants.  This allows
 creating standalone and modular security policies per application, which will
 automatically be composed between themselves according to their runtime parent
 policies.
@@ -259,18 +258,19 @@ Backward and forward compatibility
 ----------------------------------
 
 Landlock is designed to be compatible with past and future versions of the
-kernel.  This is achieved thanks to the system call attributes and the
-associated bitflags, particularly the ruleset's ``handled_access_fs``.  Making
-handled access right explicit enables the kernel and user space to have a clear
-contract with each other.  This is required to make sure sandboxing will not
-get stricter with a system update, which could break applications.
+kernel.  This is achieved bysystem call attributes and the associated bitflags,
+particularly the ``handled_access_fs`` of ruleset. Making access rights
+handling explicit enables the kernel and user space to have a clear contract
+with each other.  This is required to make sure sandboxing will not get
+stricter with a system update, which could break applications.
 
 Developers can subscribe to the `Landlock mailing list
-<https://subspace.kernel.org/lists.linux.dev.html>`_ to knowingly update and
-test their applications with the latest available features.  In the interest of
-users, and because they may use different kernel versions, it is strongly
-encouraged to follow a best-effort security approach by checking the Landlock
-ABI version at runtime and only enforcing the supported features.
+<https://subspace.kernel.org/lists.linux.dev.html>`_ to get to know with
+Landlock updates and test their applications with bleeding-edge features.
+In the interest of users, and because they may use different kernel versions,
+it is strongly encouraged to follow a best-effort security approach by
+checking the Landlock ABI version at runtime and only enforcing the supported
+features.
 
 .. _landlock_abi_versions:
 
@@ -345,7 +345,7 @@ Filesystem topology modification
 
 As for file renaming and linking, a sandboxed thread cannot modify its
 filesystem topology, whether via :manpage:`mount(2)` or
-:manpage:`pivot_root(2)`.  However, :manpage:`chroot(2)` calls are not denied.
+:manpage:`pivot_root(2)`.  However, :manpage:`chroot(2)` calls are allowed.
 
 Special filesystems
 -------------------
@@ -353,13 +353,11 @@ Special filesystems
 Access to regular files and directories can be restricted by Landlock,
 according to the handled accesses of a ruleset.  However, files that do not
 come from a user-visible filesystem (e.g. pipe, socket), but can still be
-accessed through ``/proc/<pid>/fd/*``, cannot currently be explicitly
-restricted.  Likewise, some special kernel filesystems such as nsfs, which can
-be accessed through ``/proc/<pid>/ns/*``, cannot currently be explicitly
-restricted.  However, thanks to the `ptrace restrictions`_, access to such
-sensitive ``/proc`` files are automatically restricted according to domain
-hierarchies.  Future Landlock evolutions could still enable to explicitly
-restrict such paths with dedicated ruleset flags.
+accessed through ``/proc/<pid>/fd/*``, cannot currently be restricted,
+and so some special kernel filesystems such as nsfs which can
+be accessed through ``/proc/<pid>/ns/*``. However, access to such
+sensitive ``/proc`` files are automatically restricted due to `ptrace restrictions`_ according to domain hierarchies. Future Landlock evolutions could still
+enable to explicitly restrict such paths with dedicated ruleset flags.
 
 Ruleset layers
 --------------
@@ -376,7 +374,7 @@ Memory usage
 ------------
 
 Kernel memory allocated to create rulesets is accounted and can be restricted
-by the Documentation/admin-guide/cgroup-v1/memory.rst.
+(see :ref:`cgroup-v1-memory`).
 
 Previous limitations
 ====================
@@ -386,7 +384,7 @@ File renaming and linking (ABI < 2)
 
 Because Landlock targets unprivileged access controls, it needs to properly
 handle composition of rules.  Such property also implies rules nesting.
-Properly handling multiple layers of rulesets, each one of them able to
+Properly handling multiple layers of rulesets, with each one of them able to
 restrict access to files, also implies inheritance of the ruleset restrictions
 from a parent to its hierarchy.  Because files are identified and restricted by
 their hierarchy, moving or linking a file from one directory to another implies
@@ -395,26 +393,24 @@ according to the potentially lost constraints.  To protect against privilege
 escalations through renaming or linking, and for the sake of simplicity,
 Landlock previously limited linking and renaming to the same directory.
 Starting with the Landlock ABI version 2, it is now possible to securely
-control renaming and linking thanks to the new ``LANDLOCK_ACCESS_FS_REFER``
-access right.
+control renaming and linking by ``LANDLOCK_ACCESS_FS_REFER``.
 
 .. _kernel_support:
 
 Kernel support
 ==============
 
-Landlock was first introduced in Linux 5.13 but it must be configured at build
-time with ``CONFIG_SECURITY_LANDLOCK=y``.  Landlock must also be enabled at boot
-time as the other security modules.  The list of security modules enabled by
-default is set with ``CONFIG_LSM``.  The kernel configuration should then
-contains ``CONFIG_LSM=landlock,[...]`` with ``[...]``  as the list of other
+Landlock was first introduced in Linux 5.13. To enable the support, the kernel
+must be configured at build time with ``CONFIG_SECURITY_LANDLOCK=y``. 
+Landlock must also be enabled at boot time like other security modules.
+The list of security modules enabled by default can be seen with
+``CONFIG_LSM``. The kernel configuration should then contains
+``CONFIG_LSM=landlock,[...]`` with ``[...]``  as the list of other
 potentially useful security modules for the running system (see the
 ``CONFIG_LSM`` help).
 
-If the running kernel does not have ``landlock`` in ``CONFIG_LSM``, then we can
-still enable it by adding ``lsm=landlock,[...]`` to
-Documentation/admin-guide/kernel-parameters.rst thanks to the bootloader
-configuration.
+If the running kernel does not have ``landlock`` in ``CONFIG_LSM``, then it can still be enabled by adding ``lsm=landlock,[...]`` kernel parameter (see
+:ref:`kernelparameters`)
 
 Questions and answers
 =====================
@@ -433,7 +429,7 @@ What about namespaces and containers?
 Namespaces can help create sandboxes but they are not designed for
 access-control and then miss useful features for such use case (e.g. no
 fine-grained restrictions).  Moreover, their complexity can lead to security
-issues, especially when untrusted processes can manipulate them (cf.
+issues, especially when untrusted processes can manipulate them (see
 `Controlling access to user namespaces <https://lwn.net/Articles/673597/>`_).
 
 Additional documentation
diff --git a/Documentation/userspace-api/seccomp_filter.rst b/Documentation/userspace-api/seccomp_filter.rst
index d1e2b9193f0984..a12782a958d43c 100644
--- a/Documentation/userspace-api/seccomp_filter.rst
+++ b/Documentation/userspace-api/seccomp_filter.rst
@@ -1,3 +1,5 @@
+.. _seccomp-bpf:
+
 ===========================================
 Seccomp BPF (SECure COMPuting with filters)
 ===========================================

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

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

Re: [PATCH v1 2/3] landlock: Slightly improve documentation and fix spelling

[Akira Yokosawa]

On Sat, 24 Sep 2022 20:29:24 +0700, Bagas Sanjaya wrote:
> On Fri, Sep 23, 2022 at 05:42:06PM +0200, Mickaël Salaün wrote:
>> Now that we have more than one ABI version, make limitation explanation
>> more consistent by replacing "ABI 1" with "ABI < 2".  This also
>> indicates which ABIs support such past limitation.
>> 
> 
> Hi,
> 
> I think grammar and reference link on userspace documentation can be
> improved, like:
> 
> ---- >8 ----
> 
> diff --git a/Documentation/admin-guide/cgroup-v1/memory.rst b/Documentation/admin-guide/cgroup-v1/memory.rst
> index 2cc502a75ef640..c49454900edb12 100644
> --- a/Documentation/admin-guide/cgroup-v1/memory.rst
> +++ b/Documentation/admin-guide/cgroup-v1/memory.rst
> @@ -1,3 +1,5 @@
> +.. _cgroup-v1-memory:
> +
>  ==========================
>  Memory Resource Controller
>  ==========================
> diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
> index 4c76fda076454c..dbeddb6851e6c9 100644
> --- a/Documentation/filesystems/overlayfs.rst
> +++ b/Documentation/filesystems/overlayfs.rst
> @@ -3,6 +3,8 @@
>  Written by: Neil Brown
>  Please see MAINTAINERS file for where to send questions.
>  
> +.. _overlayfs:
> +
>  Overlay Filesystem
>  ==================
>  
> diff --git a/Documentation/filesystems/sharedsubtree.rst b/Documentation/filesystems/sharedsubtree.rst
> index d83395354250d9..cbc20658bd0c07 100644
> --- a/Documentation/filesystems/sharedsubtree.rst
> +++ b/Documentation/filesystems/sharedsubtree.rst
> @@ -1,5 +1,7 @@
>  .. SPDX-License-Identifier: GPL-2.0
>  
> +.. _shared-subtrees:
> +
>  ===============
>  Shared Subtrees
>  ===============
[...]

> @@ -203,42 +204,40 @@ etc.).
>  Bind mounts and OverlayFS
>  -------------------------
>  
> -Landlock enables to restrict access to file hierarchies, which means that these
> -access rights can be propagated with bind mounts (cf.
> -Documentation/filesystems/sharedsubtree.rst) but not with
> -Documentation/filesystems/overlayfs.rst.
> +Landlock can restrict access to file hierarchies, which means that these
> +access rights can be propagated with bind mounts (see
> +:ref:`shared-subtrees`) but not with :ref:`OverlayFS <overlayfs>`.

Wait!
Bagas, your suggestion is _against_ the preference of kernel documentation.
See https://www.kernel.org/doc/html/latest/doc-guide/sphinx.html#cross-referencing

Or do you have some good reason to add labels at the beginning of
rst docs?

        Thanks, Akira

[...]

Re: [PATCH v1 2/3] landlock: Slightly improve documentation and fix spelling

[Bagas Sanjaya]

On 9/26/22 08:09, Akira Yokosawa wrote:
> Wait!
> Bagas, your suggestion is _against_ the preference of kernel documentation.
> See https://www.kernel.org/doc/html/latest/doc-guide/sphinx.html#cross-referencing
> 
> Or do you have some good reason to add labels at the beginning of
> rst docs?
> 

Ah, I don't see that :doc: directive is possible in that case, thanks.

BTW, I prefer the link text be not full path to document (like
Documentation/path/to/foo.rst), but rather either the linked doc's title
or custom text.

-- 
An old man doll... just what I always wanted! - Clara

Re: [PATCH v1 2/3] landlock: Slightly improve documentation and fix spelling

[Akira Yokosawa]

On Mon, 26 Sep 2022 08:46:18 +0700, Bagas Sanjaya wrote:
> On 9/26/22 08:09, Akira Yokosawa wrote:
>> Wait!
>> Bagas, your suggestion is _against_ the preference of kernel documentation.
>> See https://www.kernel.org/doc/html/latest/doc-guide/sphinx.html#cross-referencing
>>
>> Or do you have some good reason to add labels at the beginning of
>> rst docs?
>>
> 
> Ah, I don't see that :doc: directive is possible in that case, thanks.
> 
> BTW, I prefer the link text be not full path to document (like
> Documentation/path/to/foo.rst), but rather either the linked doc's title
> or custom text.

Again, your preference is _against_ the preference mentioned in the
cross-referencing section, quoted below:

    For most use cases, the former is preferred, as it is cleaner and more suited
    for people reading the source files. If you come across a :doc: usage that
    isn't adding any value, please feel free to convert it to just the document path.

        Thanks, Akira

Re: [PATCH v1 2/3] landlock: Slightly improve documentation and fix spelling

[Bagas Sanjaya]

On 9/26/22 11:10, Akira Yokosawa wrote:
> Again, your preference is _against_ the preference mentioned in the
> cross-referencing section, quoted below:
> 
>     For most use cases, the former is preferred, as it is cleaner and more suited
>     for people reading the source files. If you come across a :doc: usage that
>     isn't adding any value, please feel free to convert it to just the document path.
> 

OK, thanks.

-- 
An old man doll... just what I always wanted! - Clara

Re: [PATCH v1 2/3] landlock: Slightly improve documentation and fix spelling

[Mickaël Salaün]

Thanks for your patch, but this is not a review of my patch.

On 24/09/2022 15:29, Bagas Sanjaya wrote:
> On Fri, Sep 23, 2022 at 05:42:06PM +0200, Mickaël Salaün wrote:
>> Now that we have more than one ABI version, make limitation explanation
>> more consistent by replacing "ABI 1" with "ABI < 2".  This also
>> indicates which ABIs support such past limitation.
>>
> 
> Hi,
> 
> I think grammar and reference link on userspace documentation can be
> improved, like:
> 
> ---- >8 ----
> 
> diff --git a/Documentation/admin-guide/cgroup-v1/memory.rst b/Documentation/admin-guide/cgroup-v1/memory.rst
> index 2cc502a75ef640..c49454900edb12 100644
> --- a/Documentation/admin-guide/cgroup-v1/memory.rst
> +++ b/Documentation/admin-guide/cgroup-v1/memory.rst
> @@ -1,3 +1,5 @@
> +.. _cgroup-v1-memory:
> +
>   ==========================
>   Memory Resource Controller
>   ==========================
> diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
> index 4c76fda076454c..dbeddb6851e6c9 100644
> --- a/Documentation/filesystems/overlayfs.rst
> +++ b/Documentation/filesystems/overlayfs.rst
> @@ -3,6 +3,8 @@
>   Written by: Neil Brown
>   Please see MAINTAINERS file for where to send questions.
>   
> +.. _overlayfs:
> +
>   Overlay Filesystem
>   ==================
>   
> diff --git a/Documentation/filesystems/sharedsubtree.rst b/Documentation/filesystems/sharedsubtree.rst
> index d83395354250d9..cbc20658bd0c07 100644
> --- a/Documentation/filesystems/sharedsubtree.rst
> +++ b/Documentation/filesystems/sharedsubtree.rst
> @@ -1,5 +1,7 @@
>   .. SPDX-License-Identifier: GPL-2.0
>   
> +.. _shared-subtrees:
> +

These modifications are unrelated to Landlock.


>   ===============
>   Shared Subtrees
>   ===============
> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> index 5dbd577b5f58b7..13a8db59b0c0e5 100644
> --- a/Documentation/userspace-api/landlock.rst
> +++ b/Documentation/userspace-api/landlock.rst
> @@ -14,16 +14,18 @@ The goal of Landlock is to enable to restrict ambient rights (e.g. global
>   filesystem access) for a set of processes.  Because Landlock is a stackable
>   LSM, it makes possible to create safe security sandboxes as new security layers
>   in addition to the existing system-wide access-controls. This kind of sandbox
> -is expected to help mitigate the security impact of bugs or
> -unexpected/malicious behaviors in user space applications.  Landlock empowers
> -any process, including unprivileged ones, to securely restrict themselves.
> +can help mitigating the security impact of bugs or unexpected/malicious
> +behaviors in user space applications.  Landlock empowers any process, including
> +unprivileged ones, to securely restrict themselves.
>   
> -We can quickly make sure that Landlock is enabled in the running system by
> -looking for ``landlock: Up and running`` in kernel logs (as root): ``dmesg |
> -grep landlock || journalctl -kg landlock`` .  Developers can also easily check
> -for Landlock support with a :ref:`related system call <landlock_abi_versions>`.
> -If Landlock is not currently supported, we need to :ref:`configure the kernel
> -appropriately <kernel_support>`.
> +To check whether Landlock has been successfully enabled, look for
> +``landlock: Up and running`` in kernel logs, which can be seen with (as root)::
> +
> +  dmesg | grep landlock || journalctl -kg landlock
> +
> +Developers can also easily check for Landlock support with a
> +:ref:`related system call <landlock_abi_versions>`. If Landlock is not
> +currently supported, :ref:`the kernel needs to be configured <kernel_support>`.

This looks good, but it is difficult to see your changes because you 
re-wrapped the lines.


>   
>   Landlock rules
>   ==============
> @@ -36,12 +38,12 @@ the thread enforcing it, and its future children.
>   Defining and enforcing a security policy
>   ----------------------------------------
>   
> -We first need to define the ruleset that will contain our rules.  For this
> -example, the ruleset will contain rules that only allow read actions, but write
> -actions will be denied.  The ruleset then needs to handle both of these kind of
> +First, the ruleset that will contain rules needs to be defined. For this
> +example, the ruleset will contain rules that only allow read actions (write
> +actions will be denied). The ruleset then needs to handle both of these kind of

Why such changes?


>   actions.  This is required for backward and forward compatibility (i.e. the
>   kernel and user space may not know each other's supported restrictions), hence
> -the need to be explicit about the denied-by-default access rights.
> +it is required to be explicit about the denied-by-default access rights.
>   
>   .. code-block:: c
>   
> @@ -63,14 +65,14 @@ the need to be explicit about the denied-by-default access rights.
>               LANDLOCK_ACCESS_FS_REFER,
>       };
>   
> -Because we may not know on which kernel version an application will be
> -executed, it is safer to follow a best-effort security approach.  Indeed, we
> -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.
> +Because it is unknown which kernel version an application will be executed,
> +it is safer to follow a best-effort security approach. Indeed, users need to
> +be protected as much as possible whatever the kernel they are
> +using.  To avoid atomic enforcement situations (i.e. either all security
> +features or none), a dedicated Landlock command to get the current version
> +of the Landlock ABI can be used and adapt the handled accesses. Let's check if
> +``LANDLOCK_ACCESS_FS_REFER`` access right (which is available since second
> +ABI version) can be removed:

Again, difficult to review with all this line changes.


>   
>   .. code-block:: c
>   
> @@ -81,7 +83,7 @@ starting with the second version of the ABI.
>           ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_REFER;
>       }
>   
> -This enables to create an inclusive ruleset that will contain our rules.
> +This enables to create an inclusive ruleset that will contain the rules.

Can you explain your motivation?


>   
>   .. code-block:: c
>   
> @@ -93,12 +95,11 @@ This enables to create an inclusive ruleset that will contain our rules.
>           return 1;
>       }
>   
> -We can now add a new rule to this ruleset thanks to the returned file
> +Now a new rule to this ruleset can be added using the returned file
>   descriptor referring to this ruleset.  The rule will only allow reading the
> -file hierarchy ``/usr``.  Without another rule, write actions would then be
> -denied by the ruleset.  To add ``/usr`` to the ruleset, we open it with the
> -``O_PATH`` flag and fill the &struct landlock_path_beneath_attr with this file
> -descriptor.
> +file hierarchy ``/usr``. To add ``/usr`` to the ruleset, open the ruleset
> +with the ``O_PATH`` flag and fill the struct landlock_path_beneath_attr with
> +the descriptor:
>   
>   .. code-block:: c
>   
> @@ -125,15 +126,15 @@ descriptor.
>           return 1;
>       }
>   
> -It may also be required to create rules following the same logic as explained
> -for the ruleset creation, by filtering access rights according to the Landlock
> -ABI version.  In this example, this is not required because
> -``LANDLOCK_ACCESS_FS_REFER`` is not allowed by any rule.
> +It may also be required to add rules following the same logic as explained
> +above, to filter access rights according to the Landlock ABI version.
> +In this example, this is not required because ``LANDLOCK_ACCESS_FS_REFER``
> +is not allowed by any rule.
>   
> -We now have a ruleset with one rule allowing read access to ``/usr`` while
> +Now there is a ruleset with one rule allowing read access to ``/usr`` while
>   denying all other handled accesses for the filesystem.  The next step is to
> -restrict the current thread from gaining more privileges (e.g. thanks to a SUID
> -binary).
> +restrict the current thread from gaining more privileges (for example due
> +to be run by setuid binary):
>   
>   .. code-block:: c
>   
> @@ -158,7 +159,7 @@ If the ``landlock_restrict_self`` system call succeeds, the current thread is
>   now restricted and this policy will be enforced on all its subsequently created
>   children as well.  Once a thread is landlocked, there is no way to remove its
>   security policy; only adding more restrictions is allowed.  These threads are
> -now in a new Landlock domain, merge of their parent one (if any) with the new
> +now in a new Landlock domain, merging their parent one (if any) with the new

"merging their parent one"?


>   ruleset.
>   
>   Full working code can be found in `samples/landlock/sandboxer.c`_.
> @@ -166,24 +167,24 @@ Full working code can be found in `samples/landlock/sandboxer.c`_.
>   Good practices
>   --------------
>   
> -It is recommended setting access rights to file hierarchy leaves as much as
> -possible.  For instance, it is better to be able to have ``~/doc/`` as a
> -read-only hierarchy and ``~/tmp/`` as a read-write hierarchy, compared to
> -``~/`` as a read-only hierarchy and ``~/tmp/`` as a read-write hierarchy.
> -Following this good practice leads to self-sufficient hierarchies that do not
> -depend on their location (i.e. parent directories).  This is particularly
> -relevant when we want to allow linking or renaming.  Indeed, having consistent
> -access rights per directory enables to change the location of such directory
> -without relying on the destination directory access rights (except those that
> -are required for this operation, see ``LANDLOCK_ACCESS_FS_REFER``
> -documentation).
> +It is recommended to set access rights to file hierarchy leaves as much as
> +possible.  For instance, it is better to have a read-only ``~/doc/`` and
> +read-write ``~/tmp/`` hierarchies, compared to read-only ``~/`` and
> +read-write ``~/tmp/`` hierarchies. Following it leads to self-sufficient
> +hierarchies that do not depend on their location (i.e. parent directories).
> +This is particularly relevant when allowing linking or renaming is needed.
> +Indeed, having consistent access rights per directory enables to change the
> +location of such directory without relying on the destination directory access
> +rights (except those that are required for this operation, see
> +``LANDLOCK_ACCESS_FS_REFER`` documentation for example).
> +
>   Having self-sufficient hierarchies also helps to tighten the required access
>   rights to the minimal set of data.  This also helps avoid sinkhole directories,
> -i.e.  directories where data can be linked to but not linked from.  However,
> -this depends on data organization, which might not be controlled by developers.
> -In this case, granting read-write access to ``~/tmp/``, instead of write-only
> -access, would potentially allow to move ``~/tmp/`` to a non-readable directory
> -and still keep the ability to list the content of ``~/tmp/``.
> +i.e.  directories where data can be linked to but not from.  However,
> +this depends on data organization and layout, which may be outside developer
> +control. In this case, granting read-write access instead of write-only to
> +``~/tmp/`` would potentially allow to move ``~/tmp/`` to a non-readable
> +directory and still keep the ability to list the content of ``~/tmp/``.
>   
>   Layers of file path access rights
>   ---------------------------------
> @@ -194,7 +195,7 @@ the potentially other rulesets already restricting this thread.  A sandboxed
>   thread can then safely add more constraints to itself with a new enforced
>   ruleset.
>   
> -One policy layer grants access to a file path if at least one of its rules
> +A policy layer grants access to a file path if at least one of its rules
>   encountered on the path grants the access.  A sandboxed thread can only access
>   a file path if all its enforced policy layers grant the access as well as all
>   the other system access controls (e.g. filesystem DAC, other LSM policies,
> @@ -203,42 +204,40 @@ etc.).
>   Bind mounts and OverlayFS
>   -------------------------
>   
> -Landlock enables to restrict access to file hierarchies, which means that these
> -access rights can be propagated with bind mounts (cf.
> -Documentation/filesystems/sharedsubtree.rst) but not with
> -Documentation/filesystems/overlayfs.rst.
> +Landlock can restrict access to file hierarchies, which means that these
> +access rights can be propagated with bind mounts (see
> +:ref:`shared-subtrees`) but not with :ref:`OverlayFS <overlayfs>`.
>   
>   A bind mount mirrors a source file hierarchy to a destination.  The destination
>   hierarchy is then composed of the exact same files, on which Landlock rules can
> -be tied, either via the source or the destination path.  These rules restrict
> -access when they are encountered on a path, which means that they can restrict
> -access to multiple file hierarchies at the same time, whether these hierarchies
> +be applied, either via the source or the destination path.  These rules
> +restrict access when they are encountered on a path, which means that access
> +to multiple file hierarchies can be restricted, whether these hierarchies
>   are the result of bind mounts or not.
>   
> -An OverlayFS mount point consists of upper and lower layers.  These layers are
> -combined in a merge directory, result of the mount point.  This merge hierarchy
> -may include files from the upper and lower layers, but modifications performed
> -on the merge hierarchy only reflects on the upper layer.  From a Landlock
> -policy point of view, each OverlayFS layers and merge hierarchies are
> -standalone and contains their own set of files and directories, which is
> -different from bind mounts.  A policy restricting an OverlayFS layer will not
> -restrict the resulted merged hierarchy, and vice versa.  Landlock users should
> -then only think about file hierarchies they want to allow access to, regardless
> -of the underlying filesystem.
> +On the other hand, an OverlayFS mount point consists of upper and lower layers.
> +These layers are combined in a merge directory; result of the mount point.
> +This merge hierarchy may include files from the upper and lower layers, but
> +modifications performed on the merge hierarchy only reflects on the upper
> +layer. From a Landlock policy point of view, each OverlayFS layers and merge
> +hierarchies are standalone and contains their own set of files and directories,
> +which are different from bind mounts.  A policy restricting an OverlayFS layer
> +will not restrict the resulted merged hierarchy, and vice versa. Landlock users
> +should then only think about file hierarchies they want to allow access to,
> +regardless of the underlying filesystem.
>   
>   Inheritance
>   -----------
>   
>   Every new thread resulting from a :manpage:`clone(2)` inherits Landlock domain
> -restrictions from its parent.  This is similar to the seccomp inheritance (cf.
> -Documentation/userspace-api/seccomp_filter.rst) or any other LSM dealing with
> -task's :manpage:`credentials(7)`.  For instance, one process's thread may apply
> -Landlock rules to itself, but they will not be automatically applied to other
> -sibling threads (unlike POSIX thread credential changes, cf.
> -:manpage:`nptl(7)`).
> +restrictions from its parent.  This is similar to the :ref:`seccomp-bpf`) or
> +any other LSM dealing with task :manpage:`credentials(7)`. For instance, a
> +process thread may apply Landlock rules to itself, but they will not be
> +automatically applied to other sibling threads (unlike POSIX thread credential
> +changes, see :manpage:`nptl(7)`).
>   
> -When a thread sandboxes itself, we have the guarantee that the related security
> -policy will stay enforced on all this thread's descendants.  This allows
> +When a thread sandboxes itself, there is a guarantee that the related security
> +policy will stay enforced on all the thread descendants.  This allows
>   creating standalone and modular security policies per application, which will
>   automatically be composed between themselves according to their runtime parent
>   policies.
> @@ -259,18 +258,19 @@ Backward and forward compatibility
>   ----------------------------------
>   
>   Landlock is designed to be compatible with past and future versions of the
> -kernel.  This is achieved thanks to the system call attributes and the
> -associated bitflags, particularly the ruleset's ``handled_access_fs``.  Making
> -handled access right explicit enables the kernel and user space to have a clear
> -contract with each other.  This is required to make sure sandboxing will not
> -get stricter with a system update, which could break applications.
> +kernel.  This is achieved bysystem call attributes and the associated bitflags,
> +particularly the ``handled_access_fs`` of ruleset. Making access rights
> +handling explicit enables the kernel and user space to have a clear contract
> +with each other.  This is required to make sure sandboxing will not get
> +stricter with a system update, which could break applications.
>   
>   Developers can subscribe to the `Landlock mailing list
> -<https://subspace.kernel.org/lists.linux.dev.html>`_ to knowingly update and
> -test their applications with the latest available features.  In the interest of
> -users, and because they may use different kernel versions, it is strongly
> -encouraged to follow a best-effort security approach by checking the Landlock
> -ABI version at runtime and only enforcing the supported features.
> +<https://subspace.kernel.org/lists.linux.dev.html>`_ to get to know with
> +Landlock updates and test their applications with bleeding-edge features.
> +In the interest of users, and because they may use different kernel versions,
> +it is strongly encouraged to follow a best-effort security approach by
> +checking the Landlock ABI version at runtime and only enforcing the supported
> +features.
>   
>   .. _landlock_abi_versions:
>   
> @@ -345,7 +345,7 @@ Filesystem topology modification
>   
>   As for file renaming and linking, a sandboxed thread cannot modify its
>   filesystem topology, whether via :manpage:`mount(2)` or
> -:manpage:`pivot_root(2)`.  However, :manpage:`chroot(2)` calls are not denied.
> +:manpage:`pivot_root(2)`.  However, :manpage:`chroot(2)` calls are allowed.
>   
>   Special filesystems
>   -------------------
> @@ -353,13 +353,11 @@ Special filesystems
>   Access to regular files and directories can be restricted by Landlock,
>   according to the handled accesses of a ruleset.  However, files that do not
>   come from a user-visible filesystem (e.g. pipe, socket), but can still be
> -accessed through ``/proc/<pid>/fd/*``, cannot currently be explicitly
> -restricted.  Likewise, some special kernel filesystems such as nsfs, which can
> -be accessed through ``/proc/<pid>/ns/*``, cannot currently be explicitly
> -restricted.  However, thanks to the `ptrace restrictions`_, access to such
> -sensitive ``/proc`` files are automatically restricted according to domain
> -hierarchies.  Future Landlock evolutions could still enable to explicitly
> -restrict such paths with dedicated ruleset flags.
> +accessed through ``/proc/<pid>/fd/*``, cannot currently be restricted,
> +and so some special kernel filesystems such as nsfs which can
> +be accessed through ``/proc/<pid>/ns/*``. However, access to such
> +sensitive ``/proc`` files are automatically restricted due to `ptrace restrictions`_ according to domain hierarchies. Future Landlock evolutions could still
> +enable to explicitly restrict such paths with dedicated ruleset flags.
>   
>   Ruleset layers
>   --------------
> @@ -376,7 +374,7 @@ Memory usage
>   ------------
>   
>   Kernel memory allocated to create rulesets is accounted and can be restricted
> -by the Documentation/admin-guide/cgroup-v1/memory.rst.
> +(see :ref:`cgroup-v1-memory`).
>   
>   Previous limitations
>   ====================
> @@ -386,7 +384,7 @@ File renaming and linking (ABI < 2)
>   
>   Because Landlock targets unprivileged access controls, it needs to properly
>   handle composition of rules.  Such property also implies rules nesting.
> -Properly handling multiple layers of rulesets, each one of them able to
> +Properly handling multiple layers of rulesets, with each one of them able to
>   restrict access to files, also implies inheritance of the ruleset restrictions
>   from a parent to its hierarchy.  Because files are identified and restricted by
>   their hierarchy, moving or linking a file from one directory to another implies
> @@ -395,26 +393,24 @@ according to the potentially lost constraints.  To protect against privilege
>   escalations through renaming or linking, and for the sake of simplicity,
>   Landlock previously limited linking and renaming to the same directory.
>   Starting with the Landlock ABI version 2, it is now possible to securely
> -control renaming and linking thanks to the new ``LANDLOCK_ACCESS_FS_REFER``
> -access right.
> +control renaming and linking by ``LANDLOCK_ACCESS_FS_REFER``.
>   
>   .. _kernel_support:
>   
>   Kernel support
>   ==============
>   
> -Landlock was first introduced in Linux 5.13 but it must be configured at build
> -time with ``CONFIG_SECURITY_LANDLOCK=y``.  Landlock must also be enabled at boot
> -time as the other security modules.  The list of security modules enabled by
> -default is set with ``CONFIG_LSM``.  The kernel configuration should then
> -contains ``CONFIG_LSM=landlock,[...]`` with ``[...]``  as the list of other
> +Landlock was first introduced in Linux 5.13. To enable the support, the kernel
> +must be configured at build time with ``CONFIG_SECURITY_LANDLOCK=y``.
> +Landlock must also be enabled at boot time like other security modules.
> +The list of security modules enabled by default can be seen with
> +``CONFIG_LSM``. The kernel configuration should then contains
> +``CONFIG_LSM=landlock,[...]`` with ``[...]``  as the list of other
>   potentially useful security modules for the running system (see the
>   ``CONFIG_LSM`` help).
>   
> -If the running kernel does not have ``landlock`` in ``CONFIG_LSM``, then we can
> -still enable it by adding ``lsm=landlock,[...]`` to
> -Documentation/admin-guide/kernel-parameters.rst thanks to the bootloader
> -configuration.
> +If the running kernel does not have ``landlock`` in ``CONFIG_LSM``, then it can still be enabled by adding ``lsm=landlock,[...]`` kernel parameter (see
> +:ref:`kernelparameters`)
>   
>   Questions and answers
>   =====================
> @@ -433,7 +429,7 @@ What about namespaces and containers?
>   Namespaces can help create sandboxes but they are not designed for
>   access-control and then miss useful features for such use case (e.g. no
>   fine-grained restrictions).  Moreover, their complexity can lead to security
> -issues, especially when untrusted processes can manipulate them (cf.
> +issues, especially when untrusted processes can manipulate them (see
>   `Controlling access to user namespaces <https://lwn.net/Articles/673597/>`_).
>   
>   Additional documentation

This is too difficult to review and there is no explanation justifying 
them. Anyway, it is unrelated to my patch. You can send a standalone 
patch on top of this series, but I need more explanation.


> diff --git a/Documentation/userspace-api/seccomp_filter.rst b/Documentation/userspace-api/seccomp_filter.rst
> index d1e2b9193f0984..a12782a958d43c 100644
> --- a/Documentation/userspace-api/seccomp_filter.rst
> +++ b/Documentation/userspace-api/seccomp_filter.rst
> @@ -1,3 +1,5 @@
> +.. _seccomp-bpf:
> +
>   ===========================================
>   Seccomp BPF (SECure COMPuting with filters)
>   ===========================================
> 
> Thanks.
> 

Not related to Landlock.

[PATCH v1 3/3] landlock: Fix documentation style

[Mickaël Salaün]

It seems that all code should use double backquotes, which is also used
to convert "%" defines.  Let's use an homogeneous style and remove all
use of simple backquotes (which should only be used for emphasis).

Cc: Günther Noack <gnoack3000@gmail.com>
Cc: Paul Moore <paul@paul-moore.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20220923154207.3311629-4-mic@digikod.net
---
 Documentation/security/landlock.rst      |  4 +--
 Documentation/userspace-api/landlock.rst | 25 ++++++++-------
 include/uapi/linux/landlock.h            | 10 +++---
 security/landlock/syscalls.c             | 40 ++++++++++++------------
 4 files changed, 40 insertions(+), 39 deletions(-)

diff --git a/Documentation/security/landlock.rst b/Documentation/security/landlock.rst
index cc9617f3175b..c0029d5d02eb 100644
--- a/Documentation/security/landlock.rst
+++ b/Documentation/security/landlock.rst
@@ -54,8 +54,8 @@ content of a listed inode.  Indeed, a file name is local to its parent
 directory, and an inode can be referenced by multiple file names thanks to
 (hard) links.  Being able to unlink a file only has a direct impact on the
 directory, not the unlinked inode.  This is the reason why
-`LANDLOCK_ACCESS_FS_REMOVE_FILE` or `LANDLOCK_ACCESS_FS_REFER` are not allowed
-to be tied to files but only to directories.
+``LANDLOCK_ACCESS_FS_REMOVE_FILE`` or ``LANDLOCK_ACCESS_FS_REFER`` are not
+allowed to be tied to files but only to directories.
 
 Tests
 =====
diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index 83bae71bf042..cec780c2f497 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -69,7 +69,7 @@ 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
+remove the ``LANDLOCK_ACCESS_FS_REFER`` access right which is only supported
 starting with the second version of the ABI.
 
 .. code-block:: c
@@ -128,7 +128,7 @@ descriptor.
 It may also be required to create rules following the same logic as explained
 for the ruleset creation, by filtering access rights according to the Landlock
 ABI version.  In this example, this is not required because
-`LANDLOCK_ACCESS_FS_REFER` is not allowed by any rule.
+``LANDLOCK_ACCESS_FS_REFER`` is not allowed by any rule.
 
 We now have a ruleset with one rule allowing read access to ``/usr`` while
 denying all other handled accesses for the filesystem.  The next step is to
@@ -154,8 +154,8 @@ The current thread is now ready to sandbox itself with the ruleset.
     }
     close(ruleset_fd);
 
-If the `landlock_restrict_self` system call succeeds, the current thread is now
-restricted and this policy will be enforced on all its subsequently created
+If the ``landlock_restrict_self`` system call succeeds, the current thread is
+now restricted and this policy will be enforced on all its subsequently created
 children as well.  Once a thread is landlocked, there is no way to remove its
 security policy; only adding more restrictions is allowed.  These threads are
 now in a new Landlock domain, merge of their parent one (if any) with the new
@@ -175,7 +175,8 @@ depend on their location (i.e. parent directories).  This is particularly
 relevant when we want to allow linking or renaming.  Indeed, having consistent
 access rights per directory enables to change the location of such directory
 without relying on the destination directory access rights (except those that
-are required for this operation, see `LANDLOCK_ACCESS_FS_REFER` documentation).
+are required for this operation, see ``LANDLOCK_ACCESS_FS_REFER``
+documentation).
 Having self-sufficient hierarchies also helps to tighten the required access
 rights to the minimal set of data.  This also helps avoid sinkhole directories,
 i.e.  directories where data can be linked to but not linked from.  However,
@@ -259,7 +260,7 @@ Backward and forward compatibility
 
 Landlock is designed to be compatible with past and future versions of the
 kernel.  This is achieved thanks to the system call attributes and the
-associated bitflags, particularly the ruleset's `handled_access_fs`.  Making
+associated bitflags, particularly the ruleset's ``handled_access_fs``.  Making
 handled access right explicit enables the kernel and user space to have a clear
 contract with each other.  This is required to make sure sandboxing will not
 get stricter with a system update, which could break applications.
@@ -394,7 +395,7 @@ according to the potentially lost constraints.  To protect against privilege
 escalations through renaming or linking, and for the sake of simplicity,
 Landlock previously limited linking and renaming to the same directory.
 Starting with the Landlock ABI version 2, it is now possible to securely
-control renaming and linking thanks to the new `LANDLOCK_ACCESS_FS_REFER`
+control renaming and linking thanks to the new ``LANDLOCK_ACCESS_FS_REFER``
 access right.
 
 .. _kernel_support:
@@ -403,14 +404,14 @@ Kernel support
 ==============
 
 Landlock was first introduced in Linux 5.13 but it must be configured at build
-time with `CONFIG_SECURITY_LANDLOCK=y`.  Landlock must also be enabled at boot
+time with ``CONFIG_SECURITY_LANDLOCK=y``.  Landlock must also be enabled at boot
 time as the other security modules.  The list of security modules enabled by
-default is set with `CONFIG_LSM`.  The kernel configuration should then
-contains `CONFIG_LSM=landlock,[...]` with `[...]`  as the list of other
+default is set with ``CONFIG_LSM``.  The kernel configuration should then
+contains ``CONFIG_LSM=landlock,[...]`` with ``[...]``  as the list of other
 potentially useful security modules for the running system (see the
-`CONFIG_LSM` help).
+``CONFIG_LSM`` help).
 
-If the running kernel does not have `landlock` in `CONFIG_LSM`, then we can
+If the running kernel does not have ``landlock`` in ``CONFIG_LSM``, then we can
 still enable it by adding ``lsm=landlock,[...]`` to
 Documentation/admin-guide/kernel-parameters.rst thanks to the bootloader
 configuration.
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index 23df4e0e8ace..9c4bcc37a455 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -26,7 +26,7 @@ struct landlock_ruleset_attr {
 	 * Landlock filesystem access rights that are not part of
 	 * handled_access_fs are allowed.  This is needed for backward
 	 * compatibility reasons.  One exception is the
-	 * LANDLOCK_ACCESS_FS_REFER access right, which is always implicitly
+	 * %LANDLOCK_ACCESS_FS_REFER access right, which is always implicitly
 	 * handled, but must still be explicitly handled to add new rules with
 	 * this access right.
 	 */
@@ -128,11 +128,11 @@ struct landlock_path_beneath_attr {
  *   hierarchy must also always have the same or a superset of restrictions of
  *   the source hierarchy.  If it is not the case, or if the domain doesn't
  *   handle this access right, such actions are denied by default with errno
- *   set to EXDEV.  Linking also requires a LANDLOCK_ACCESS_FS_MAKE_* access
- *   right on the destination directory, and renaming also requires a
- *   LANDLOCK_ACCESS_FS_REMOVE_* access right on the source's (file or
+ *   set to ``EXDEV``.  Linking also requires a ``LANDLOCK_ACCESS_FS_MAKE_*``
+ *   access right on the destination directory, and renaming also requires a
+ *   ``LANDLOCK_ACCESS_FS_REMOVE_*`` access right on the source's (file or
  *   directory) parent.  Otherwise, such actions are denied with errno set to
- *   EACCES.  The EACCES errno prevails over EXDEV to let user space
+ *   ``EACCES``.  The ``EACCES`` errno prevails over ``EXDEV`` to let user space
  *   efficiently deal with an unrecoverable error.
  *
  * .. warning::
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 735a0865ea11..2ca0ccbd905a 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -149,10 +149,10 @@ static const struct file_operations ruleset_fops = {
  *
  * Possible returned errors are:
  *
- * - EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
- * - EINVAL: unknown @flags, or unknown access, or too small @size;
- * - E2BIG or EFAULT: @attr or @size inconsistencies;
- * - ENOMSG: empty &landlock_ruleset_attr.handled_access_fs.
+ * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
+ * - %EINVAL: unknown @flags, or unknown access, or too small @size;
+ * - %E2BIG or %EFAULT: @attr or @size inconsistencies;
+ * - %ENOMSG: empty &landlock_ruleset_attr.handled_access_fs.
  */
 SYSCALL_DEFINE3(landlock_create_ruleset,
 		const struct landlock_ruleset_attr __user *const, attr,
@@ -280,7 +280,7 @@ static int get_path_from_fd(const s32 fd, struct path *const path)
  * @ruleset_fd: File descriptor tied to the ruleset that should be extended
  *		with the new rule.
  * @rule_type: Identify the structure type pointed to by @rule_attr (only
- *             LANDLOCK_RULE_PATH_BENEATH for now).
+ *             %LANDLOCK_RULE_PATH_BENEATH for now).
  * @rule_attr: Pointer to a rule (only of type &struct
  *             landlock_path_beneath_attr for now).
  * @flags: Must be 0.
@@ -290,17 +290,17 @@ static int get_path_from_fd(const s32 fd, struct path *const path)
  *
  * Possible returned errors are:
  *
- * - EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
- * - EINVAL: @flags is not 0, or inconsistent access in the rule (i.e.
+ * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
+ * - %EINVAL: @flags is not 0, or inconsistent access in the rule (i.e.
  *   &landlock_path_beneath_attr.allowed_access is not a subset of the
  *   ruleset handled accesses);
- * - ENOMSG: Empty accesses (e.g. &landlock_path_beneath_attr.allowed_access);
- * - EBADF: @ruleset_fd is not a file descriptor for the current thread, or a
+ * - %ENOMSG: Empty accesses (e.g. &landlock_path_beneath_attr.allowed_access);
+ * - %EBADF: @ruleset_fd is not a file descriptor for the current thread, or a
  *   member of @rule_attr is not a file descriptor as expected;
- * - EBADFD: @ruleset_fd is not a ruleset file descriptor, or a member of
+ * - %EBADFD: @ruleset_fd is not a ruleset file descriptor, or a member of
  *   @rule_attr is not the expected file descriptor type;
- * - EPERM: @ruleset_fd has no write access to the underlying ruleset;
- * - EFAULT: @rule_attr inconsistency.
+ * - %EPERM: @ruleset_fd has no write access to the underlying ruleset;
+ * - %EFAULT: @rule_attr inconsistency.
  */
 SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
 		const enum landlock_rule_type, rule_type,
@@ -378,20 +378,20 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
  * @flags: Must be 0.
  *
  * This system call enables to enforce a Landlock ruleset on the current
- * thread.  Enforcing a ruleset requires that the task has CAP_SYS_ADMIN in its
+ * thread.  Enforcing a ruleset requires that the task has %CAP_SYS_ADMIN in its
  * namespace or is running with no_new_privs.  This avoids scenarios where
  * unprivileged tasks can affect the behavior of privileged children.
  *
  * Possible returned errors are:
  *
- * - EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
- * - EINVAL: @flags is not 0.
- * - EBADF: @ruleset_fd is not a file descriptor for the current thread;
- * - EBADFD: @ruleset_fd is not a ruleset file descriptor;
- * - EPERM: @ruleset_fd has no read access to the underlying ruleset, or the
+ * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
+ * - %EINVAL: @flags is not 0.
+ * - %EBADF: @ruleset_fd is not a file descriptor for the current thread;
+ * - %EBADFD: @ruleset_fd is not a ruleset file descriptor;
+ * - %EPERM: @ruleset_fd has no read access to the underlying ruleset, or the
  *   current thread is not running with no_new_privs, or it doesn't have
- *   CAP_SYS_ADMIN in its namespace.
- * - E2BIG: The maximum number of stacked rulesets is reached for the current
+ *   %CAP_SYS_ADMIN in its namespace.
+ * - %E2BIG: The maximum number of stacked rulesets is reached for the current
  *   thread.
  */
 SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
-- 
2.37.2

Re: [PATCH v1 3/3] landlock: Fix documentation style

[Günther Noack]

On Fri, Sep 23, 2022 at 05:42:07PM +0200, Mickaël Salaün wrote:
> It seems that all code should use double backquotes, which is also used
> to convert "%" defines.  Let's use an homogeneous style and remove all
> use of simple backquotes (which should only be used for emphasis).
> 
> Cc: Günther Noack <gnoack3000@gmail.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Link: https://lore.kernel.org/r/20220923154207.3311629-4-mic@digikod.net
> ---
>  Documentation/security/landlock.rst      |  4 +--
>  Documentation/userspace-api/landlock.rst | 25 ++++++++-------
>  include/uapi/linux/landlock.h            | 10 +++---
>  security/landlock/syscalls.c             | 40 ++++++++++++------------
>  4 files changed, 40 insertions(+), 39 deletions(-)
> 
> diff --git a/Documentation/security/landlock.rst b/Documentation/security/landlock.rst
> index cc9617f3175b..c0029d5d02eb 100644
> --- a/Documentation/security/landlock.rst
> +++ b/Documentation/security/landlock.rst
> @@ -54,8 +54,8 @@ content of a listed inode.  Indeed, a file name is local to its parent
>  directory, and an inode can be referenced by multiple file names thanks to
>  (hard) links.  Being able to unlink a file only has a direct impact on the
>  directory, not the unlinked inode.  This is the reason why
> -`LANDLOCK_ACCESS_FS_REMOVE_FILE` or `LANDLOCK_ACCESS_FS_REFER` are not allowed
> -to be tied to files but only to directories.
> +``LANDLOCK_ACCESS_FS_REMOVE_FILE`` or ``LANDLOCK_ACCESS_FS_REFER`` are not
> +allowed to be tied to files but only to directories.
>  
>  Tests
>  =====
> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> index 83bae71bf042..cec780c2f497 100644
> --- a/Documentation/userspace-api/landlock.rst
> +++ b/Documentation/userspace-api/landlock.rst
> @@ -69,7 +69,7 @@ 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
> +remove the ``LANDLOCK_ACCESS_FS_REFER`` access right which is only supported
>  starting with the second version of the ABI.
>  
>  .. code-block:: c
> @@ -128,7 +128,7 @@ descriptor.
>  It may also be required to create rules following the same logic as explained
>  for the ruleset creation, by filtering access rights according to the Landlock
>  ABI version.  In this example, this is not required because
> -`LANDLOCK_ACCESS_FS_REFER` is not allowed by any rule.
> +``LANDLOCK_ACCESS_FS_REFER`` is not allowed by any rule.
>  
>  We now have a ruleset with one rule allowing read access to ``/usr`` while
>  denying all other handled accesses for the filesystem.  The next step is to
> @@ -154,8 +154,8 @@ The current thread is now ready to sandbox itself with the ruleset.
>      }
>      close(ruleset_fd);
>  
> -If the `landlock_restrict_self` system call succeeds, the current thread is now
> -restricted and this policy will be enforced on all its subsequently created
> +If the ``landlock_restrict_self`` system call succeeds, the current thread is
> +now restricted and this policy will be enforced on all its subsequently created
>  children as well.  Once a thread is landlocked, there is no way to remove its
>  security policy; only adding more restrictions is allowed.  These threads are
>  now in a new Landlock domain, merge of their parent one (if any) with the new
> @@ -175,7 +175,8 @@ depend on their location (i.e. parent directories).  This is particularly
>  relevant when we want to allow linking or renaming.  Indeed, having consistent
>  access rights per directory enables to change the location of such directory
>  without relying on the destination directory access rights (except those that
> -are required for this operation, see `LANDLOCK_ACCESS_FS_REFER` documentation).
> +are required for this operation, see ``LANDLOCK_ACCESS_FS_REFER``
> +documentation).
>  Having self-sufficient hierarchies also helps to tighten the required access
>  rights to the minimal set of data.  This also helps avoid sinkhole directories,
>  i.e.  directories where data can be linked to but not linked from.  However,
> @@ -259,7 +260,7 @@ Backward and forward compatibility
>  
>  Landlock is designed to be compatible with past and future versions of the
>  kernel.  This is achieved thanks to the system call attributes and the
> -associated bitflags, particularly the ruleset's `handled_access_fs`.  Making
> +associated bitflags, particularly the ruleset's ``handled_access_fs``.  Making
>  handled access right explicit enables the kernel and user space to have a clear
>  contract with each other.  This is required to make sure sandboxing will not
>  get stricter with a system update, which could break applications.
> @@ -394,7 +395,7 @@ according to the potentially lost constraints.  To protect against privilege
>  escalations through renaming or linking, and for the sake of simplicity,
>  Landlock previously limited linking and renaming to the same directory.
>  Starting with the Landlock ABI version 2, it is now possible to securely
> -control renaming and linking thanks to the new `LANDLOCK_ACCESS_FS_REFER`
> +control renaming and linking thanks to the new ``LANDLOCK_ACCESS_FS_REFER``
>  access right.
>  
>  .. _kernel_support:
> @@ -403,14 +404,14 @@ Kernel support
>  ==============
>  
>  Landlock was first introduced in Linux 5.13 but it must be configured at build
> -time with `CONFIG_SECURITY_LANDLOCK=y`.  Landlock must also be enabled at boot
> +time with ``CONFIG_SECURITY_LANDLOCK=y``.  Landlock must also be enabled at boot
>  time as the other security modules.  The list of security modules enabled by
> -default is set with `CONFIG_LSM`.  The kernel configuration should then
> -contains `CONFIG_LSM=landlock,[...]` with `[...]`  as the list of other
> +default is set with ``CONFIG_LSM``.  The kernel configuration should then
> +contains ``CONFIG_LSM=landlock,[...]`` with ``[...]``  as the list of other
>  potentially useful security modules for the running system (see the
> -`CONFIG_LSM` help).
> +``CONFIG_LSM`` help).
>  
> -If the running kernel does not have `landlock` in `CONFIG_LSM`, then we can
> +If the running kernel does not have ``landlock`` in ``CONFIG_LSM``, then we can
>  still enable it by adding ``lsm=landlock,[...]`` to
>  Documentation/admin-guide/kernel-parameters.rst thanks to the bootloader
>  configuration.
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index 23df4e0e8ace..9c4bcc37a455 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -26,7 +26,7 @@ struct landlock_ruleset_attr {
>  	 * Landlock filesystem access rights that are not part of
>  	 * handled_access_fs are allowed.  This is needed for backward
>  	 * compatibility reasons.  One exception is the
> -	 * LANDLOCK_ACCESS_FS_REFER access right, which is always implicitly
> +	 * %LANDLOCK_ACCESS_FS_REFER access right, which is always implicitly

At the risk of asking a newbie question, why is this access right
prefixed with % in this command, but most others are surrounded by
double-backticks in other places?

According to [1], % is used to denote a constant - should these
LANDLOCK_ACCESS_FS* rights not all be showing up as constants?

[1] https://docs.kernel.org/doc-guide/kernel-doc.html#highlights-and-cross-references

Apart from this item, this change looks good.

>  	 * handled, but must still be explicitly handled to add new rules with
>  	 * this access right.
>  	 */
> @@ -128,11 +128,11 @@ struct landlock_path_beneath_attr {
>   *   hierarchy must also always have the same or a superset of restrictions of
>   *   the source hierarchy.  If it is not the case, or if the domain doesn't
>   *   handle this access right, such actions are denied by default with errno
> - *   set to EXDEV.  Linking also requires a LANDLOCK_ACCESS_FS_MAKE_* access
> - *   right on the destination directory, and renaming also requires a
> - *   LANDLOCK_ACCESS_FS_REMOVE_* access right on the source's (file or
> + *   set to ``EXDEV``.  Linking also requires a ``LANDLOCK_ACCESS_FS_MAKE_*``
> + *   access right on the destination directory, and renaming also requires a
> + *   ``LANDLOCK_ACCESS_FS_REMOVE_*`` access right on the source's (file or
>   *   directory) parent.  Otherwise, such actions are denied with errno set to
> - *   EACCES.  The EACCES errno prevails over EXDEV to let user space
> + *   ``EACCES``.  The ``EACCES`` errno prevails over ``EXDEV`` to let user space
>   *   efficiently deal with an unrecoverable error.
>   *
>   * .. warning::
> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> index 735a0865ea11..2ca0ccbd905a 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -149,10 +149,10 @@ static const struct file_operations ruleset_fops = {
>   *
>   * Possible returned errors are:
>   *
> - * - EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
> - * - EINVAL: unknown @flags, or unknown access, or too small @size;
> - * - E2BIG or EFAULT: @attr or @size inconsistencies;
> - * - ENOMSG: empty &landlock_ruleset_attr.handled_access_fs.
> + * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
> + * - %EINVAL: unknown @flags, or unknown access, or too small @size;
> + * - %E2BIG or %EFAULT: @attr or @size inconsistencies;
> + * - %ENOMSG: empty &landlock_ruleset_attr.handled_access_fs.
>   */
>  SYSCALL_DEFINE3(landlock_create_ruleset,
>  		const struct landlock_ruleset_attr __user *const, attr,
> @@ -280,7 +280,7 @@ static int get_path_from_fd(const s32 fd, struct path *const path)
>   * @ruleset_fd: File descriptor tied to the ruleset that should be extended
>   *		with the new rule.
>   * @rule_type: Identify the structure type pointed to by @rule_attr (only
> - *             LANDLOCK_RULE_PATH_BENEATH for now).
> + *             %LANDLOCK_RULE_PATH_BENEATH for now).
>   * @rule_attr: Pointer to a rule (only of type &struct
>   *             landlock_path_beneath_attr for now).
>   * @flags: Must be 0.
> @@ -290,17 +290,17 @@ static int get_path_from_fd(const s32 fd, struct path *const path)
>   *
>   * Possible returned errors are:
>   *
> - * - EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
> - * - EINVAL: @flags is not 0, or inconsistent access in the rule (i.e.
> + * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
> + * - %EINVAL: @flags is not 0, or inconsistent access in the rule (i.e.
>   *   &landlock_path_beneath_attr.allowed_access is not a subset of the
>   *   ruleset handled accesses);
> - * - ENOMSG: Empty accesses (e.g. &landlock_path_beneath_attr.allowed_access);
> - * - EBADF: @ruleset_fd is not a file descriptor for the current thread, or a
> + * - %ENOMSG: Empty accesses (e.g. &landlock_path_beneath_attr.allowed_access);
> + * - %EBADF: @ruleset_fd is not a file descriptor for the current thread, or a
>   *   member of @rule_attr is not a file descriptor as expected;
> - * - EBADFD: @ruleset_fd is not a ruleset file descriptor, or a member of
> + * - %EBADFD: @ruleset_fd is not a ruleset file descriptor, or a member of
>   *   @rule_attr is not the expected file descriptor type;
> - * - EPERM: @ruleset_fd has no write access to the underlying ruleset;
> - * - EFAULT: @rule_attr inconsistency.
> + * - %EPERM: @ruleset_fd has no write access to the underlying ruleset;
> + * - %EFAULT: @rule_attr inconsistency.
>   */
>  SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
>  		const enum landlock_rule_type, rule_type,
> @@ -378,20 +378,20 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
>   * @flags: Must be 0.
>   *
>   * This system call enables to enforce a Landlock ruleset on the current
> - * thread.  Enforcing a ruleset requires that the task has CAP_SYS_ADMIN in its
> + * thread.  Enforcing a ruleset requires that the task has %CAP_SYS_ADMIN in its
>   * namespace or is running with no_new_privs.  This avoids scenarios where
>   * unprivileged tasks can affect the behavior of privileged children.
>   *
>   * Possible returned errors are:
>   *
> - * - EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
> - * - EINVAL: @flags is not 0.
> - * - EBADF: @ruleset_fd is not a file descriptor for the current thread;
> - * - EBADFD: @ruleset_fd is not a ruleset file descriptor;
> - * - EPERM: @ruleset_fd has no read access to the underlying ruleset, or the
> + * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
> + * - %EINVAL: @flags is not 0.
> + * - %EBADF: @ruleset_fd is not a file descriptor for the current thread;
> + * - %EBADFD: @ruleset_fd is not a ruleset file descriptor;
> + * - %EPERM: @ruleset_fd has no read access to the underlying ruleset, or the
>   *   current thread is not running with no_new_privs, or it doesn't have
> - *   CAP_SYS_ADMIN in its namespace.
> - * - E2BIG: The maximum number of stacked rulesets is reached for the current
> + *   %CAP_SYS_ADMIN in its namespace.
> + * - %E2BIG: The maximum number of stacked rulesets is reached for the current
>   *   thread.
>   */
>  SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
> -- 
> 2.37.2
> 

-- 

Re: [PATCH v1 3/3] landlock: Fix documentation style

[Mickaël Salaün]

On 23/09/2022 23:18, Günther Noack wrote:
> On Fri, Sep 23, 2022 at 05:42:07PM +0200, Mickaël Salaün wrote:
>> It seems that all code should use double backquotes, which is also used
>> to convert "%" defines.  Let's use an homogeneous style and remove all
>> use of simple backquotes (which should only be used for emphasis).
>>
>> Cc: Günther Noack <gnoack3000@gmail.com>
>> Cc: Paul Moore <paul@paul-moore.com>
>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>> Link: https://lore.kernel.org/r/20220923154207.3311629-4-mic@digikod.net
>> ---
>>   Documentation/security/landlock.rst      |  4 +--
>>   Documentation/userspace-api/landlock.rst | 25 ++++++++-------
>>   include/uapi/linux/landlock.h            | 10 +++---
>>   security/landlock/syscalls.c             | 40 ++++++++++++------------
>>   4 files changed, 40 insertions(+), 39 deletions(-)
>>
>> diff --git a/Documentation/security/landlock.rst b/Documentation/security/landlock.rst
>> index cc9617f3175b..c0029d5d02eb 100644
>> --- a/Documentation/security/landlock.rst
>> +++ b/Documentation/security/landlock.rst
>> @@ -54,8 +54,8 @@ content of a listed inode.  Indeed, a file name is local to its parent
>>   directory, and an inode can be referenced by multiple file names thanks to
>>   (hard) links.  Being able to unlink a file only has a direct impact on the
>>   directory, not the unlinked inode.  This is the reason why
>> -`LANDLOCK_ACCESS_FS_REMOVE_FILE` or `LANDLOCK_ACCESS_FS_REFER` are not allowed
>> -to be tied to files but only to directories.
>> +``LANDLOCK_ACCESS_FS_REMOVE_FILE`` or ``LANDLOCK_ACCESS_FS_REFER`` are not
>> +allowed to be tied to files but only to directories.
>>   
>>   Tests
>>   =====
>> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
>> index 83bae71bf042..cec780c2f497 100644
>> --- a/Documentation/userspace-api/landlock.rst
>> +++ b/Documentation/userspace-api/landlock.rst
>> @@ -69,7 +69,7 @@ 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
>> +remove the ``LANDLOCK_ACCESS_FS_REFER`` access right which is only supported
>>   starting with the second version of the ABI.
>>   
>>   .. code-block:: c
>> @@ -128,7 +128,7 @@ descriptor.
>>   It may also be required to create rules following the same logic as explained
>>   for the ruleset creation, by filtering access rights according to the Landlock
>>   ABI version.  In this example, this is not required because
>> -`LANDLOCK_ACCESS_FS_REFER` is not allowed by any rule.
>> +``LANDLOCK_ACCESS_FS_REFER`` is not allowed by any rule.
>>   
>>   We now have a ruleset with one rule allowing read access to ``/usr`` while
>>   denying all other handled accesses for the filesystem.  The next step is to
>> @@ -154,8 +154,8 @@ The current thread is now ready to sandbox itself with the ruleset.
>>       }
>>       close(ruleset_fd);
>>   
>> -If the `landlock_restrict_self` system call succeeds, the current thread is now
>> -restricted and this policy will be enforced on all its subsequently created
>> +If the ``landlock_restrict_self`` system call succeeds, the current thread is
>> +now restricted and this policy will be enforced on all its subsequently created
>>   children as well.  Once a thread is landlocked, there is no way to remove its
>>   security policy; only adding more restrictions is allowed.  These threads are
>>   now in a new Landlock domain, merge of their parent one (if any) with the new
>> @@ -175,7 +175,8 @@ depend on their location (i.e. parent directories).  This is particularly
>>   relevant when we want to allow linking or renaming.  Indeed, having consistent
>>   access rights per directory enables to change the location of such directory
>>   without relying on the destination directory access rights (except those that
>> -are required for this operation, see `LANDLOCK_ACCESS_FS_REFER` documentation).
>> +are required for this operation, see ``LANDLOCK_ACCESS_FS_REFER``
>> +documentation).
>>   Having self-sufficient hierarchies also helps to tighten the required access
>>   rights to the minimal set of data.  This also helps avoid sinkhole directories,
>>   i.e.  directories where data can be linked to but not linked from.  However,
>> @@ -259,7 +260,7 @@ Backward and forward compatibility
>>   
>>   Landlock is designed to be compatible with past and future versions of the
>>   kernel.  This is achieved thanks to the system call attributes and the
>> -associated bitflags, particularly the ruleset's `handled_access_fs`.  Making
>> +associated bitflags, particularly the ruleset's ``handled_access_fs``.  Making
>>   handled access right explicit enables the kernel and user space to have a clear
>>   contract with each other.  This is required to make sure sandboxing will not
>>   get stricter with a system update, which could break applications.
>> @@ -394,7 +395,7 @@ according to the potentially lost constraints.  To protect against privilege
>>   escalations through renaming or linking, and for the sake of simplicity,
>>   Landlock previously limited linking and renaming to the same directory.
>>   Starting with the Landlock ABI version 2, it is now possible to securely
>> -control renaming and linking thanks to the new `LANDLOCK_ACCESS_FS_REFER`
>> +control renaming and linking thanks to the new ``LANDLOCK_ACCESS_FS_REFER``
>>   access right.
>>   
>>   .. _kernel_support:
>> @@ -403,14 +404,14 @@ Kernel support
>>   ==============
>>   
>>   Landlock was first introduced in Linux 5.13 but it must be configured at build
>> -time with `CONFIG_SECURITY_LANDLOCK=y`.  Landlock must also be enabled at boot
>> +time with ``CONFIG_SECURITY_LANDLOCK=y``.  Landlock must also be enabled at boot
>>   time as the other security modules.  The list of security modules enabled by
>> -default is set with `CONFIG_LSM`.  The kernel configuration should then
>> -contains `CONFIG_LSM=landlock,[...]` with `[...]`  as the list of other
>> +default is set with ``CONFIG_LSM``.  The kernel configuration should then
>> +contains ``CONFIG_LSM=landlock,[...]`` with ``[...]``  as the list of other
>>   potentially useful security modules for the running system (see the
>> -`CONFIG_LSM` help).
>> +``CONFIG_LSM`` help).
>>   
>> -If the running kernel does not have `landlock` in `CONFIG_LSM`, then we can
>> +If the running kernel does not have ``landlock`` in ``CONFIG_LSM``, then we can
>>   still enable it by adding ``lsm=landlock,[...]`` to
>>   Documentation/admin-guide/kernel-parameters.rst thanks to the bootloader
>>   configuration.
>> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
>> index 23df4e0e8ace..9c4bcc37a455 100644
>> --- a/include/uapi/linux/landlock.h
>> +++ b/include/uapi/linux/landlock.h
>> @@ -26,7 +26,7 @@ struct landlock_ruleset_attr {
>>   	 * Landlock filesystem access rights that are not part of
>>   	 * handled_access_fs are allowed.  This is needed for backward
>>   	 * compatibility reasons.  One exception is the
>> -	 * LANDLOCK_ACCESS_FS_REFER access right, which is always implicitly
>> +	 * %LANDLOCK_ACCESS_FS_REFER access right, which is always implicitly
> 
> At the risk of asking a newbie question, why is this access right
> prefixed with % in this command, but most others are surrounded by
> double-backticks in other places?

The % command is only interpreted in code comments, but it in ignored in 
RST files. This translates to double-backticks though. It seems that % 
is more appropriate this way and it is also more readable. I only used 
`` when I couldn't use %.

Jonathan may have a better explanation.

> 
> According to [1], % is used to denote a constant - should these
> LANDLOCK_ACCESS_FS* rights not all be showing up as constants?
> 
> [1] https://docs.kernel.org/doc-guide/kernel-doc.html#highlights-and-cross-references
> 
> Apart from this item, this change looks good.
> 
>>   	 * handled, but must still be explicitly handled to add new rules with
>>   	 * this access right.
>>   	 */
>> @@ -128,11 +128,11 @@ struct landlock_path_beneath_attr {
>>    *   hierarchy must also always have the same or a superset of restrictions of
>>    *   the source hierarchy.  If it is not the case, or if the domain doesn't
>>    *   handle this access right, such actions are denied by default with errno
>> - *   set to EXDEV.  Linking also requires a LANDLOCK_ACCESS_FS_MAKE_* access
>> - *   right on the destination directory, and renaming also requires a
>> - *   LANDLOCK_ACCESS_FS_REMOVE_* access right on the source's (file or
>> + *   set to ``EXDEV``.  Linking also requires a ``LANDLOCK_ACCESS_FS_MAKE_*``
>> + *   access right on the destination directory, and renaming also requires a
>> + *   ``LANDLOCK_ACCESS_FS_REMOVE_*`` access right on the source's (file or
>>    *   directory) parent.  Otherwise, such actions are denied with errno set to
>> - *   EACCES.  The EACCES errno prevails over EXDEV to let user space
>> + *   ``EACCES``.  The ``EACCES`` errno prevails over ``EXDEV`` to let user space
>>    *   efficiently deal with an unrecoverable error.
>>    *
>>    * .. warning::
>> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
>> index 735a0865ea11..2ca0ccbd905a 100644
>> --- a/security/landlock/syscalls.c
>> +++ b/security/landlock/syscalls.c
>> @@ -149,10 +149,10 @@ static const struct file_operations ruleset_fops = {
>>    *
>>    * Possible returned errors are:
>>    *
>> - * - EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
>> - * - EINVAL: unknown @flags, or unknown access, or too small @size;
>> - * - E2BIG or EFAULT: @attr or @size inconsistencies;
>> - * - ENOMSG: empty &landlock_ruleset_attr.handled_access_fs.
>> + * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
>> + * - %EINVAL: unknown @flags, or unknown access, or too small @size;
>> + * - %E2BIG or %EFAULT: @attr or @size inconsistencies;
>> + * - %ENOMSG: empty &landlock_ruleset_attr.handled_access_fs.
>>    */
>>   SYSCALL_DEFINE3(landlock_create_ruleset,
>>   		const struct landlock_ruleset_attr __user *const, attr,
>> @@ -280,7 +280,7 @@ static int get_path_from_fd(const s32 fd, struct path *const path)
>>    * @ruleset_fd: File descriptor tied to the ruleset that should be extended
>>    *		with the new rule.
>>    * @rule_type: Identify the structure type pointed to by @rule_attr (only
>> - *             LANDLOCK_RULE_PATH_BENEATH for now).
>> + *             %LANDLOCK_RULE_PATH_BENEATH for now).
>>    * @rule_attr: Pointer to a rule (only of type &struct
>>    *             landlock_path_beneath_attr for now).
>>    * @flags: Must be 0.
>> @@ -290,17 +290,17 @@ static int get_path_from_fd(const s32 fd, struct path *const path)
>>    *
>>    * Possible returned errors are:
>>    *
>> - * - EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
>> - * - EINVAL: @flags is not 0, or inconsistent access in the rule (i.e.
>> + * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
>> + * - %EINVAL: @flags is not 0, or inconsistent access in the rule (i.e.
>>    *   &landlock_path_beneath_attr.allowed_access is not a subset of the
>>    *   ruleset handled accesses);
>> - * - ENOMSG: Empty accesses (e.g. &landlock_path_beneath_attr.allowed_access);
>> - * - EBADF: @ruleset_fd is not a file descriptor for the current thread, or a
>> + * - %ENOMSG: Empty accesses (e.g. &landlock_path_beneath_attr.allowed_access);
>> + * - %EBADF: @ruleset_fd is not a file descriptor for the current thread, or a
>>    *   member of @rule_attr is not a file descriptor as expected;
>> - * - EBADFD: @ruleset_fd is not a ruleset file descriptor, or a member of
>> + * - %EBADFD: @ruleset_fd is not a ruleset file descriptor, or a member of
>>    *   @rule_attr is not the expected file descriptor type;
>> - * - EPERM: @ruleset_fd has no write access to the underlying ruleset;
>> - * - EFAULT: @rule_attr inconsistency.
>> + * - %EPERM: @ruleset_fd has no write access to the underlying ruleset;
>> + * - %EFAULT: @rule_attr inconsistency.
>>    */
>>   SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
>>   		const enum landlock_rule_type, rule_type,
>> @@ -378,20 +378,20 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
>>    * @flags: Must be 0.
>>    *
>>    * This system call enables to enforce a Landlock ruleset on the current
>> - * thread.  Enforcing a ruleset requires that the task has CAP_SYS_ADMIN in its
>> + * thread.  Enforcing a ruleset requires that the task has %CAP_SYS_ADMIN in its
>>    * namespace or is running with no_new_privs.  This avoids scenarios where
>>    * unprivileged tasks can affect the behavior of privileged children.
>>    *
>>    * Possible returned errors are:
>>    *
>> - * - EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
>> - * - EINVAL: @flags is not 0.
>> - * - EBADF: @ruleset_fd is not a file descriptor for the current thread;
>> - * - EBADFD: @ruleset_fd is not a ruleset file descriptor;
>> - * - EPERM: @ruleset_fd has no read access to the underlying ruleset, or the
>> + * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
>> + * - %EINVAL: @flags is not 0.
>> + * - %EBADF: @ruleset_fd is not a file descriptor for the current thread;
>> + * - %EBADFD: @ruleset_fd is not a ruleset file descriptor;
>> + * - %EPERM: @ruleset_fd has no read access to the underlying ruleset, or the
>>    *   current thread is not running with no_new_privs, or it doesn't have
>> - *   CAP_SYS_ADMIN in its namespace.
>> - * - E2BIG: The maximum number of stacked rulesets is reached for the current
>> + *   %CAP_SYS_ADMIN in its namespace.
>> + * - %E2BIG: The maximum number of stacked rulesets is reached for the current
>>    *   thread.
>>    */
>>   SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
>> -- 
>> 2.37.2
>>
> 

Re: [PATCH v1 3/3] landlock: Fix documentation style

[Bagas Sanjaya]

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

On Fri, Sep 23, 2022 at 05:42:07PM +0200, Mickaël Salaün wrote:
> It seems that all code should use double backquotes, which is also used
> to convert "%" defines.  Let's use an homogeneous style and remove all
> use of simple backquotes (which should only be used for emphasis).
> 

A little nit: the kernel log content at the opening paragraph should
also be in inline code:

---- >8 ----
diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index cec780c2f4973c..5dbd577b5f58b7 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -19,10 +19,10 @@ unexpected/malicious behaviors in user space applications.  Landlock empowers
 any process, including unprivileged ones, to securely restrict themselves.
 
 We can quickly make sure that Landlock is enabled in the running system by
-looking for "landlock: Up and running" in kernel logs (as root): ``dmesg | grep
-landlock || journalctl -kg landlock`` .  Developers can also easily check for
-Landlock support with a :ref:`related system call <landlock_abi_versions>`.  If
-Landlock is not currently supported, we need to :ref:`configure the kernel
+looking for ``landlock: Up and running`` in kernel logs (as root): ``dmesg | 
+grep landlock || journalctl -kg landlock`` .  Developers can also easily check
+for Landlock support with a :ref:`related system call <landlock_abi_versions>`.
+If Landlock is not currently supported, we need to :ref:`configure the kernel
 appropriately <kernel_support>`.
 
 Landlock rules

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

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

Re: [PATCH v1 3/3] landlock: Fix documentation style

[Mickaël Salaün]

On 24/09/2022 11:50, Bagas Sanjaya wrote:
> On Fri, Sep 23, 2022 at 05:42:07PM +0200, Mickaël Salaün wrote:
>> It seems that all code should use double backquotes, which is also used
>> to convert "%" defines.  Let's use an homogeneous style and remove all
>> use of simple backquotes (which should only be used for emphasis).
>>
> 
> A little nit: the kernel log content at the opening paragraph should
> also be in inline code:

Is there some precedent? Explicit quotes (") seems more appropriate 
because we are talking about a text string (not a command nor a code 
snippet).

> 
> ---- >8 ----
> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> index cec780c2f4973c..5dbd577b5f58b7 100644
> --- a/Documentation/userspace-api/landlock.rst
> +++ b/Documentation/userspace-api/landlock.rst
> @@ -19,10 +19,10 @@ unexpected/malicious behaviors in user space applications.  Landlock empowers
>   any process, including unprivileged ones, to securely restrict themselves.
>   
>   We can quickly make sure that Landlock is enabled in the running system by
> -looking for "landlock: Up and running" in kernel logs (as root): ``dmesg | grep
> -landlock || journalctl -kg landlock`` .  Developers can also easily check for
> -Landlock support with a :ref:`related system call <landlock_abi_versions>`.  If
> -Landlock is not currently supported, we need to :ref:`configure the kernel
> +looking for ``landlock: Up and running`` in kernel logs (as root): ``dmesg |
> +grep landlock || journalctl -kg landlock`` .  Developers can also easily check
> +for Landlock support with a :ref:`related system call <landlock_abi_versions>`.
> +If Landlock is not currently supported, we need to :ref:`configure the kernel
>   appropriately <kernel_support>`.
>   
>   Landlock rules
> 
> Thanks.
>