Re: [PATCH bpf-next v5 5/7] bpf: lsm: Initialize the BPF LSM hooks

On 23-Mär 12:44, Kees Cook wrote:

On Mon, Mar 23, 2020 at 05:44:13PM +0100, KP Singh wrote:

quoted

From: KP Singh <redacted>

The bpf_lsm_ nops are initialized into the LSM framework like any other
LSM.  Some LSM hooks do not have 0 as their default return value. The
__weak symbol for these hooks is overridden by a corresponding
definition in security/bpf/hooks.c

The LSM can be enabled / disabled with CONFIG_LSM.

Signed-off-by: KP Singh <redacted>

Nice! This is super clean on the LSM side of things. :)

One note below...

quoted

Reviewed-by: Brendan Jackman <redacted>

[...]

quoted

+
+/*
+ * Copyright (C) 2020 Google LLC.
+ */
+#include <linux/lsm_hooks.h>
+#include <linux/bpf_lsm.h>
+
+/* Some LSM hooks do not have 0 as their default return values. Override the
+ * __weak definitons generated by default for these hooks

If you wanted to avoid this, couldn't you make the default return value
part of lsm_hooks.h?

e.g.:

LSM_HOOK(int, -EOPNOTSUPP, inode_getsecurity, struct inode *inode,
	 const char *name, void **buffer, bool alloc)

...

#define LSM_HOOK(RET, DEFAULT, NAME, ...)	\
	LSM_HOOK_##RET(NAME, DEFAULT, __VA_ARGS__)
...
#define LSM_HOOK_int(NAME, DEFAULT, ...)	\
noinline int bpf_lsm_##NAME(__VA_ARGS__)	\
{						\
	return (DEFAULT);			\
}

Then all the __weak stuff is gone, and the following 4 functions don't
need to be written out, and the information is available to the macros
if anyone else might ever want it.

Thanks, I like it!

If no-one objects, I will update it in the next revision.

- KP

-Kees

quoted

+ */
+noinline int bpf_lsm_inode_getsecurity(struct inode *inode, const char *name,
+				       void **buffer, bool alloc)
+};

[...]

quoted

-- 
2.20.1

-- 
Kees Cook