On Fri, Dec 20, 2019 at 7:43 AM KP Singh <kpsingh@xxxxxxxxxxxx> wrote:
>
> From: KP Singh <kpsingh@xxxxxxxxxx>
>
> The LSM creates files in securityfs for each hook registered with the
> LSM.
>
> /sys/kernel/security/bpf/<h_name>
>
> The list of LSM hooks are maintained in an internal header "hooks.h"
> Eventually, this list should either be defined collectively in
> include/linux/lsm_hooks.h or auto-generated from it.
>
> * Creation of a file for the hook in the securityfs.
> * Allocation of a bpf_lsm_hook data structure which stores
> a pointer to the dentry of the newly created file in securityfs.
> * Creation of a typedef for the hook so that BTF information
> can be generated for the LSM hooks to:
>
> - Make them "Compile Once, Run Everywhere".
> - Pass the right arguments when the attached programs are run.
> - Verify the accesses made by the program by using the BTF
> information.
>
> Signed-off-by: KP Singh <kpsingh@xxxxxxxxxx>
> ---
> include/linux/bpf_lsm.h | 12 +
> security/bpf/Makefile | 4 +-
> security/bpf/include/bpf_lsm.h | 63 ++
> security/bpf/include/fs.h | 23 +
> security/bpf/include/hooks.h | 1015 ++++++++++++++++++++++++++++++++
> security/bpf/lsm.c | 138 ++++-
> security/bpf/lsm_fs.c | 82 +++
> 7 files changed, 1333 insertions(+), 4 deletions(-)
> create mode 100644 include/linux/bpf_lsm.h
> create mode 100644 security/bpf/include/bpf_lsm.h
> create mode 100644 security/bpf/include/fs.h
> create mode 100644 security/bpf/include/hooks.h
> create mode 100644 security/bpf/lsm_fs.c
>
[...]
> +
> +/*
> + * The hooks can have an int or void return type, these macros allow having a
> + * single implementation of DEFINE_LSM_HOOK irrespective of the return type.
> + */
> +#define LSM_HOOK_RET(ret, x) LSM_HOOK_RET_##ret(x)
> +#define LSM_HOOK_RET_int(x) x
> +#define LSM_HOOK_RET_void(x)
> +
> +/*
> + * This macro defines the body of a LSM hook which runs the eBPF programs that
> + * are attached to the hook and returns the error code from the eBPF programs if
> + * the return type of the hook is int.
> + */
> +#define DEFINE_LSM_HOOK(hook, ret, proto, args) \
> +typedef ret (*lsm_btf_##hook)(proto); \
> +static ret bpf_lsm_##hook(proto) \
> +{ \
> + return LSM_HOOK_RET(ret, LSM_RUN_PROGS(hook##_type, args)); \
> }
I'm probably missing something, but according to LSM_HOOK_RET
definition for when ret==void, bpf_lsm_##hook will be a noop and won't
call any BPF program. Did I miss some additional macro magic?
>
> +/*
> + * Define the body of each of the LSM hooks defined in hooks.h.
> + */
> +#define BPF_LSM_HOOK(hook, ret, args, proto) \
> + DEFINE_LSM_HOOK(hook, ret, BPF_LSM_ARGS(args), BPF_LSM_ARGS(proto))
> +#include "hooks.h"
> +#undef BPF_LSM_HOOK
> +#undef DEFINE_LSM_HOOK
> +
> +/*
> + * Initialize the bpf_lsm_hooks_list for each of the hooks defined in hooks.h.
> + * The list contains information for each of the hook and can be indexed by the
> + * its type to initialize security FS, attach, detach and execute eBPF programs
> + * for the hook.
> + */
> +struct bpf_lsm_hook bpf_lsm_hooks_list[] = {
> + #define BPF_LSM_HOOK(h, ...) \
> + [h##_type] = { \
> + .h_type = h##_type, \
> + .mutex = __MUTEX_INITIALIZER( \
> + bpf_lsm_hooks_list[h##_type].mutex), \
> + .name = #h, \
> + .btf_hook_func = \
> + (void *)(lsm_btf_##h)(bpf_lsm_##h), \
this btf_hook_func, is it assigned just so that type information for
bpf_lsm_xxx typedefs are preserved, is that right? It doesn't seem to
be ever called or read. If I'm not missing anything, check out
Martin's latest STRUCT_OPS patch set. He defines EMIT_TYPE_INFO(type)
macro, which will ensure that BTF for specified type is emitted into
vmlinux BTF, without actually using any extra space, defining extra
fields or static variables, etc. I suggest using the same for the
cleanest result.
One more thing regarding lsm_bpf_ typedefs. Currently you are defining
them as a pointer to func_proto, matching LSM hook. There is an
alternative approach, which has few benefits over using func_proto. If
instead you define a struct, where each argument of func prototype is
represented as 8-byte aligned field, this will contain all the
necessary information for BPF verifier to do its job (just like
func_proto). But in addition to that, when vmlinux.h is generated, it
will contain a nice struct bpf_lsm_<hook_name> with correct structure
to be used **directly** in BPF program, as a single context argument.
So with vmlinux.h, users won't have to re-define all the argument
types and names in their BPF_TRACE_x definition. Let me provide
concrete example from your cover letter. This is what you provide as
an example:
BPF_TRACE_3("lsm/file_mprotect", mprotect_audit,
struct vm_area_struct *, vma,
unsigned long, reqprot, unsigned long, prot) {...}
on kernel side, you'll have:
typedef int (*bpf_lsm_file_mprotect)(struct vm_area_struct *vma,
unsigned long reqprot,
unsigned long prot);
So you can see that user has to go and copy/paste all the arguments
and their types and paste them in this verbose BPF_TRACE_3 macro to
define correct BPF program.
Now, imagine that instead of typedef above, we define equivalent struct:
struct bpf_lsm_file_mprotect {
struct vm_area_struct *vma;
unsigned long reqprot;
unsigned long prot;
};
This type will get dumped into vmlinux.h, which can be used from BPF
user code as such:
SEC("lsm/file_mprotect")
int mprotect_audito(struct bpf_lsm_file_mprotect *ctx)
{
... here you can use ctx->vma, ctx->reqprot, ctx->prot ...
}
Meanwhile, there will be just minimal changes to BPF verifier to use
such struct instead of func_proto for verification of LSM programs.
We currently have similar issue with raw_tp programs and I've been
thinking about switching that to structs instead of func_proto, so we
might as well coordinate that and reuse the same logic in BPF
verifier.
Thoughts?
> + },
> + #include "hooks.h"
> + #undef BPF_LSM_HOOK
> +};
> +
[...]
