Re: [PATCH v6 5/5] landlock: Document Landlock's file truncation support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/09/2022 17:46, Günther Noack wrote:
On Fri, Sep 09, 2022 at 03:51:35PM +0200, Mickaël Salaün wrote:

On 08/09/2022 21:58, Günther Noack wrote:
Use the LANDLOCK_ACCESS_FS_TRUNCATE flag in the tutorial.

Adapt the backwards compatibility example and discussion to remove the
truncation flag where needed.

Point out potential surprising behaviour related to truncate.

Signed-off-by: Günther Noack <gnoack3000@xxxxxxxxx>
---
   Documentation/userspace-api/landlock.rst | 62 +++++++++++++++++++++---
   1 file changed, 54 insertions(+), 8 deletions(-)

diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index b8ea59493964..57802fd1e09b 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
@@ -60,7 +60,8 @@ the need to be explicit about the denied-by-default access rights.
               LANDLOCK_ACCESS_FS_MAKE_FIFO |
               LANDLOCK_ACCESS_FS_MAKE_BLOCK |
               LANDLOCK_ACCESS_FS_MAKE_SYM |
-            LANDLOCK_ACCESS_FS_REFER,
+            LANDLOCK_ACCESS_FS_REFER |
+            LANDLOCK_ACCESS_FS_TRUNCATE,
       };
   Because we may not know on which kernel version an application will be
@@ -69,16 +70,26 @@ 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.
+remove the `LANDLOCK_ACCESS_FS_REFER` or `LANDLOCK_ACCESS_FS_TRUNCATE` access
+rights, which are only supported starting with the second and third version of
+the ABI.
   .. code-block:: c
       int abi;
       abi = landlock_create_ruleset(NULL, 0, LANDLOCK_CREATE_RULESET_VERSION);
-    if (abi < 2) {
-        ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_REFER;
+    switch (abi) {
+    case -1:
+            perror("The running kernel does not enable to use Landlock");
+            return 1;
+    case 1:
+            /* Removes LANDLOCK_ACCESS_FS_REFER for ABI < 2 */
+            ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_REFER;
+            __attribute__((fallthrough));
+    case 2:
+            /* Removes LANDLOCK_ACCESS_FS_TRUNCATE for ABI < 3 */
+            ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_TRUNCATE;
       }
   This enables to create an inclusive ruleset that will contain our rules.
@@ -127,8 +138,8 @@ 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.
+ABI version.  In this example, this is not required because all of the requested
+``allowed_access`` rights are already available in ABI 1.

This fix is correct, but it should not be part of this series. FYI, I have a
patch almost ready to fix some documentation style issues. Please remove
this hunk for the next series. I'll deal with the merge conflicts if any.

Can you please clarify what part of it should not be part of this
series?

My mistake, I guess I was reviewing something else… I was thinking about style changes, but it is not the case here. Using "``" is correct.



In this hunk, I've started using double backquote, but I've also
changed the meaning of the sentence slightly so that it is still
correct when the truncate right is introduced.

It is still correct that the backwards compatibility check is not
required because LANDLOCK_ACCESS_FS_REFER is not allowed by any rule.
But with the new truncate flag, LANDLOCK_ACCESS_FS_TRUNCATE may also
not be allowed by any rule so that we can skip this check.

Should I remove this hunk entirely?

Keep your changes, it's better like this.



Or maybe rather phrase it like

   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` and
   `LANDLOCK_ACCESS_FS_TRUNCATE` are not allowed by any rule.
?



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux