Skip to contentSkip to conversation

landlock.7: Code example improvements

PATCH In review Günther Noack 3 participants 7 messages Apr 19, 2023 – Apr 20, 2023 Man-pages: manual pages for Linux -- Sections 2, 3, 4, 5, and 7

lore
Share & tools
lore

Günther Noack

author
#0 lore raw

cover landlock.7: Code example improvements

Various improvements as pointed out by Mickaël Salaün in
https://lore.kernel.org/linux-man/[email protected]/

* Checking system call results differently, for consistency
* Use constants for the compatibility table
(I'm not very attached to the other solution)
* Better wording for error message if Landlock unusable
* Return instead of exit() if Landlock unusable

Regarding the EOPNOTSUPP/ENOTSUP discussion, the consensus seems to be that EOPNOTSUPP is preferable, because that is the only of the two error codes that the kernel knows about.

Günther Noack (4):
landlock.7: Check syscall result with == -1 instead of <= 0 landlock.7: Use LANDLOCK_* constants for compatibility table landlock.7: wfix: Error message wording in code example landlock.7: Return instead of exit() if Landlock is unusable

 man7/landlock.7 | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

base-commit: 6693a21cf73c502f2429b4ec07698130a2be9a93

Signature · 1 line
2.40.0
author
#1 lore raw

PATCH 1/4 landlock.7: Check syscall result with == -1 instead of <= 0

As pointed out by Mickaël Salaün in
https://lore.kernel.org/linux-man/[email protected]/

Signed-off-by Günther Noack <[email protected]>
---
 man7/landlock.7 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
man7/landlock.7 +1 -1
index 16feef42c..fc4a95031 100644
@@ -445,7 +445,7 @@ __u64 landlock_fs_access_rights[] = {
 
 int abi = landlock_create_ruleset(NULL, 0,
                                   LANDLOCK_CREATE_RULESET_VERSION);
-if (abi <= 0) {
+if (abi == \-1) {
     /*
      * Kernel too old, not compiled with Landlock,
      * or Landlock was not enabled at boot time.
Signature · 1 line
2.40.0
author
#2 lore raw

PATCH 2/4 landlock.7: Use LANDLOCK_* constants for compatibility table

In reply to Günther Noack

As discussed in
https://lore.kernel.org/linux-man/[email protected]/

Signed-off-by Günther Noack <[email protected]>
---
 man7/landlock.7 | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
man7/landlock.7 +3 -3
index fc4a95031..cb8a4f62e 100644
@@ -438,9 +438,9 @@ and only use the available subset of access rights:
  * numbers hardcoded to keep the example short.
  */
 __u64 landlock_fs_access_rights[] = {
-    (1ULL << 13) \- 1,  /* ABI v1                 */
-    (1ULL << 14) \- 1,  /* ABI v2: add "refer"    */
-    (1ULL << 15) \- 1,  /* ABI v3: add "truncate" */
+    (LANDLOCK_ACCESS_FS_MAKE_SYM << 1) \- 1,  /* v1                 */
+    (LANDLOCK_ACCESS_FS_REFER    << 1) \- 1,  /* v2: add "refer"    */
+    (LANDLOCK_ACCESS_FS_TRUNCATE << 1) \- 1,  /* v3: add "truncate" */
 };
 
 int abi = landlock_create_ruleset(NULL, 0,
Signature · 1 line
2.40.0
author
#3 lore raw

PATCH 3/4 landlock.7: wfix: Error message wording in code example

In reply to Günther Noack

As suggested by Mickaël Salaün in
https://lore.kernel.org/linux-man/[email protected]/

Signed-off-by Günther Noack <[email protected]>
---
 man7/landlock.7 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
man7/landlock.7 +1 -1
index cb8a4f62e..7067c058c 100644
@@ -450,7 +450,7 @@ if (abi == \-1) {
      * Kernel too old, not compiled with Landlock,
      * or Landlock was not enabled at boot time.
      */
-    perror("Giving up \- No Landlock support");
+    perror("Unable to use Landlock");
     exit(EXIT_FAILURE);
 }
 abi = MIN(abi, 3);
Signature · 1 line
2.40.0
author
#4 lore raw

PATCH 4/4 landlock.7: Return instead of exit() if Landlock is unusable

In reply to Günther Noack

When following a best effort approach,
we should not fail when Landlock is unusable,
but we should fall back to doing nothing.

As discussed in
https://lore.kernel.org/linux-man/[email protected]/

Signed-off-by Günther Noack <[email protected]>
---
 man7/landlock.7 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
man7/landlock.7 +1 -1
index 7067c058c..b6c9d3821 100644
@@ -451,7 +451,7 @@ if (abi == \-1) {
      * or Landlock was not enabled at boot time.
      */
     perror("Unable to use Landlock");
-    exit(EXIT_FAILURE);
+    return;  /* Graceful fallback: Do nothing. */
 }
 abi = MIN(abi, 3);
Signature · 1 line
2.40.0
#5 lore raw
In reply to Günther Noack

Hi Günther!

Quoted 13 lines · Günther Noack

Various improvements as pointed out by Mickaël Salaün in
https://lore.kernel.org/linux-man/[email protected]/

* Checking system call results differently, for consistency
* Use constants for the compatibility table
(I'm not very attached to the other solution)
* Better wording for error message if Landlock unusable
* Return instead of exit() if Landlock unusable

Regarding the EOPNOTSUPP/ENOTSUP discussion, the consensus seems to be that EOPNOTSUPP is preferable, because that is the only of the two error codes that the kernel knows about.

Quoted 5 lines

Günther Noack (4):
landlock.7: Check syscall result with == -1 instead of <= 0 landlock.7: Use LANDLOCK_* constants for compatibility table landlock.7: wfix: Error message wording in code example landlock.7: Return instead of exit() if Landlock is unusable

Thanks! Patch set applied. (I tweaked the commit messages a little bit.)

Cheers,
Alex

Quoted 6 lines
 man7/landlock.7 | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

base-commit: 6693a21cf73c502f2429b4ec07698130a2be9a93

Signature · 2 lines
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
#6 lore raw
Quoted 16 lines · Alejandro Colomar

Hi Günther!

Quoted 13 lines · Günther Noack

Various improvements as pointed out by Mickaël Salaün in
https://lore.kernel.org/linux-man/[email protected]/

* Checking system call results differently, for consistency
* Use constants for the compatibility table
(I'm not very attached to the other solution)
* Better wording for error message if Landlock unusable
* Return instead of exit() if Landlock unusable

Regarding the EOPNOTSUPP/ENOTSUP discussion, the consensus seems to be that EOPNOTSUPP is preferable, because that is the only of the two error codes that the kernel knows about.

Quoted 8 lines
Quoted 5 lines

Günther Noack (4):
landlock.7: Check syscall result with == -1 instead of <= 0 landlock.7: Use LANDLOCK_* constants for compatibility table landlock.7: wfix: Error message wording in code example landlock.7: Return instead of exit() if Landlock is unusable

Thanks! Patch set applied. (I tweaked the commit messages a little bit.)

Thanks Günther and Alex!

Quoted 11 lines

Cheers,
Alex

Quoted 6 lines
  man7/landlock.7 | 12 ++++++------
  1 file changed, 6 insertions(+), 6 deletions(-)

base-commit: 6693a21cf73c502f2429b4ec07698130a2be9a93