Thread (76 messages) 76 messages, 9 authors, 2016-10-19

Re: [RFC v3 07/22] landlock: Handle file comparisons

From: Kees Cook <hidden>
Date: 2016-10-03 23:30:33
Also in: linux-api, lkml, netdev

On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Add eBPF functions to compare file system access with a Landlock file
system handle:
* bpf_landlock_cmp_fs_prop_with_struct_file(prop, map, map_op, file)
  This function allows to compare the dentry, inode, device or mount
  point of the currently accessed file, with a reference handle.
* bpf_landlock_cmp_fs_beneath_with_struct_file(opt, map, map_op, file)
  This function allows an eBPF program to check if the current accessed
  file is the same or in the hierarchy of a reference handle.

The goal of file system handle is to abstract kernel objects such as a
struct file or a struct inode. Userland can create this kind of handle
thanks to the BPF_MAP_UPDATE_ELEM command. The element is a struct
landlock_handle containing the handle type (e.g.
BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD) and a file descriptor. This could
also be any descriptions able to match a struct file or a struct inode
(e.g. path or glob string).

Changes since v2:
* add MNT_INTERNAL check to only add file handle from user-visible FS
  (e.g. no anonymous inode)
* replace struct file* with struct path* in map_landlock_handle
* add BPF protos
* fix bpf_landlock_cmp_fs_prop_with_struct_file()

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: James Morris <redacted>
Cc: Kees Cook <redacted>
Cc: Serge E. Hallyn <serge@hallyn.com>
Link: https://lkml.kernel.org/r/CALCETrWwTiz3kZTkEgOW24-DvhQq6LftwEXh77FD2G5o71yD7g@mail.gmail.com
---
 include/linux/bpf.h            |  10 +++
 include/uapi/linux/bpf.h       |  49 +++++++++++
 kernel/bpf/arraymap.c          |  21 +++++
 kernel/bpf/verifier.c          |   8 ++
 security/landlock/Makefile     |   2 +-
 security/landlock/checker_fs.c | 179 +++++++++++++++++++++++++++++++++++++++++
 security/landlock/checker_fs.h |  20 +++++
 security/landlock/lsm.c        |   6 ++
 8 files changed, 294 insertions(+), 1 deletion(-)
 create mode 100644 security/landlock/checker_fs.c
 create mode 100644 security/landlock/checker_fs.h
[...]
diff --git a/security/landlock/checker_fs.c b/security/landlock/checker_fs.c
new file mode 100644
index 000000000000..39eb85dc7d18
--- /dev/null
+++ b/security/landlock/checker_fs.c
@@ -0,0 +1,179 @@
+/*
+ * Landlock LSM - File System Checkers
+ *
+ * Copyright (C) 2016  Mickaël Salaün <mic@digikod.net>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2, as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/bpf.h> /* enum bpf_map_array_op */
+#include <linux/errno.h>
+#include <linux/fs.h> /* path_is_under() */
+#include <linux/path.h> /* struct path */
+
+#include "checker_fs.h"
+
+#define EQUAL_NOT_NULL(a, b) (a && a == b)
+
+/*
+ * bpf_landlock_cmp_fs_prop_with_struct_file
+ *
+ * Cf. include/uapi/linux/bpf.h
+ */
+static inline u64 bpf_landlock_cmp_fs_prop_with_struct_file(u64 r1_property,
+               u64 r2_map, u64 r3_map_op, u64 r4_file, u64 r5)
+{
+       u8 property = (u8) r1_property;
+       struct bpf_map *map = (struct bpf_map *) (unsigned long) r2_map;
+       enum bpf_map_array_op map_op = r3_map_op;
+       struct file *file = (struct file *) (unsigned long) r4_file;
+       struct bpf_array *array = container_of(map, struct bpf_array, map);
+       struct path *p1, *p2;
+       struct map_landlock_handle *handle;
+       int i;
+
+       /* ARG_CONST_PTR_TO_LANDLOCK_HANDLE_FS is an arraymap */
+       if (unlikely(!map)) {
+               WARN_ON(1);
+               return -EFAULT;
+       }
Just some minor style/readability nits...

This is more readable as:

if (WARN_ON(!map))
    return -EFAULT;

(WARN_ON already includes the unlikely() and passes through the test result.)
+       if (unlikely(!file))
+               return -ENOENT;
+       if (unlikely((property | _LANDLOCK_FLAG_FS_MASK) != _LANDLOCK_FLAG_FS_MASK))
+               return -EINVAL;
+
+       /* for now, only handle OP_OR */
+       switch (map_op) {
+       case BPF_MAP_ARRAY_OP_OR:
+               break;
+       case BPF_MAP_ARRAY_OP_UNSPEC:
+       case BPF_MAP_ARRAY_OP_AND:
+       case BPF_MAP_ARRAY_OP_XOR:
+       default:
+               return -EINVAL;
+       }
+       p2 = &file->f_path;
+
+       synchronize_rcu();
+
+       for (i = 0; i < array->n_entries; i++) {
+               bool result_dentry = !(property & LANDLOCK_FLAG_FS_DENTRY);
+               bool result_inode = !(property & LANDLOCK_FLAG_FS_INODE);
+               bool result_device = !(property & LANDLOCK_FLAG_FS_DEVICE);
+               bool result_mount = !(property & LANDLOCK_FLAG_FS_MOUNT);
+
+               handle = (struct map_landlock_handle *)
+                               (array->value + array->elem_size * i);
+
+               if (handle->type != BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD) {
+                       WARN_ON(1);
+                       return -EFAULT;
+               }
Same here... and in the other function (much of which seems to repeat
-- can some of these checks be put into common functions?)

-Kees

-- 
Kees Cook
Nexus Security
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help