[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 3 Oct 2016 16:30:21 -0700
From: Kees Cook <keescook@...omium.org>
To: Mickaël Salaün <mic@...ikod.net>
Cc: LKML <linux-kernel@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
Andy Lutomirski <luto@...capital.net>, Arnd Bergmann <arnd@...db.de>,
Casey Schaufler <casey@...aufler-ca.com>, Daniel Borkmann <daniel@...earbox.net>,
Daniel Mack <daniel@...que.org>, David Drysdale <drysdale@...gle.com>,
"David S . Miller" <davem@...emloft.net>, Elena Reshetova <elena.reshetova@...el.com>,
"Eric W . Biederman" <ebiederm@...ssion.com>, James Morris <james.l.morris@...cle.com>,
Paul Moore <pmoore@...hat.com>, Sargun Dhillon <sargun@...gun.me>,
"Serge E . Hallyn" <serge@...lyn.com>, Tejun Heo <tj@...nel.org>, Will Drewry <wad@...omium.org>,
"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>, Linux API <linux-api@...r.kernel.org>,
linux-security-module <linux-security-module@...r.kernel.org>,
Network Development <netdev@...r.kernel.org>, Cgroups <cgroups@...r.kernel.org>
Subject: Re: [RFC v3 07/22] landlock: Handle file comparisons
On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün <mic@...ikod.net> wrote:
> 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@...ikod.net>
> Cc: Alexei Starovoitov <ast@...nel.org>
> Cc: Andy Lutomirski <luto@...capital.net>
> Cc: Daniel Borkmann <daniel@...earbox.net>
> Cc: David S. Miller <davem@...emloft.net>
> Cc: James Morris <james.l.morris@...cle.com>
> Cc: Kees Cook <keescook@...omium.org>
> Cc: Serge E. Hallyn <serge@...lyn.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@...ikod.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
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
