On 29/10/2020 02:06, Jann Horn wrote:

(On Tue, Oct 27, 2020 at 9:04 PM Mickaël Salaün [off-list ref] wrote:

quoted

diff --git a/security/landlock/fs.c b/security/landlock/fs.c

[...]

quoted

+static inline u32 get_file_access(const struct file *const file)
+{
+       u32 access = 0;
+
+       if (file->f_mode & FMODE_READ) {
+               /* A directory can only be opened in read mode. */
+               if (S_ISDIR(file_inode(file)->i_mode))
+                       return LANDLOCK_ACCESS_FS_READ_DIR;
+               access = LANDLOCK_ACCESS_FS_READ_FILE;
+       }
+       /*
+        * A LANDLOCK_ACCESS_FS_APPEND could be added but we also need to check
+        * fcntl(2).
+        */

Once https://lore.kernel.org/linux-api/20200831153207.GO3265@brightrain.aerifal.cx/ (local)
lands, pwritev2() with RWF_NOAPPEND will also be problematic for
classifying "write" vs "append"; you may want to include that in the
comment. (Or delete the comment.)

Contrary to fcntl(2), pwritev2(2) doesn't seems to modify the file
description. Otherwise, other LSMs would need to be patched.
I'll remove this comment anyway.

quoted

+       if (file->f_mode & FMODE_WRITE)
+               access |= LANDLOCK_ACCESS_FS_WRITE_FILE;
+       /* __FMODE_EXEC is indeed part of f_flags, not f_mode. */
+       if (file->f_flags & __FMODE_EXEC)
+               access |= LANDLOCK_ACCESS_FS_EXECUTE;
+       return access;
+}

[...]