From c3dd6e94df7193f33f45d33303f5e85afb2a72dc Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Thu, 30 Nov 2023 10:52:21 -0800 Subject: bpf,lsm: refactor bpf_prog_alloc/bpf_prog_free LSM hooks Based on upstream discussion ([0]), rework existing bpf_prog_alloc_security LSM hook. Rename it to bpf_prog_load and instead of passing bpf_prog_aux, pass proper bpf_prog pointer for a full BPF program struct. Also, we pass bpf_attr union with all the user-provided arguments for BPF_PROG_LOAD command. This will give LSMs as much information as we can basically provide. The hook is also BPF token-aware now, and optional bpf_token struct is passed as a third argument. bpf_prog_load LSM hook is called after a bunch of sanity checks were performed, bpf_prog and bpf_prog_aux were allocated and filled out, but right before performing full-fledged BPF verification step. bpf_prog_free LSM hook is now accepting struct bpf_prog argument, for consistency. SELinux code is adjusted to all new names, types, and signatures. Note, given that bpf_prog_load (previously bpf_prog_alloc) hook can be used by some LSMs to allocate extra security blob, but also by other LSMs to reject BPF program loading, we need to make sure that bpf_prog_free LSM hook is called after bpf_prog_load/bpf_prog_alloc one *even* if the hook itself returned error. If we don't do that, we run the risk of leaking memory. This seems to be possible today when combining SELinux and BPF LSM, as one example, depending on their relative ordering. Also, for BPF LSM setup, add bpf_prog_load and bpf_prog_free to sleepable LSM hooks list, as they are both executed in sleepable context. Also drop bpf_prog_load hook from untrusted, as there is no issue with refcount or anything else anymore, that originally forced us to add it to untrusted list in c0c852dd1876 ("bpf: Do not mark certain LSM hook arguments as trusted"). We now trigger this hook much later and it should not be an issue anymore. [0] https://lore.kernel.org/bpf/9fe88aef7deabbe87d3fc38c4aea3c69.paul@paul-moore.com/ Acked-by: Paul Moore Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/r/20231130185229.2688956-10-andrii@kernel.org Signed-off-by: Alexei Starovoitov --- security/selinux/hooks.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index feda711c6b7b..eabee39e983c 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6805,7 +6805,8 @@ static void selinux_bpf_map_free(struct bpf_map *map) kfree(bpfsec); } -static int selinux_bpf_prog_alloc(struct bpf_prog_aux *aux) +static int selinux_bpf_prog_load(struct bpf_prog *prog, union bpf_attr *attr, + struct bpf_token *token) { struct bpf_security_struct *bpfsec; @@ -6814,16 +6815,16 @@ static int selinux_bpf_prog_alloc(struct bpf_prog_aux *aux) return -ENOMEM; bpfsec->sid = current_sid(); - aux->security = bpfsec; + prog->aux->security = bpfsec; return 0; } -static void selinux_bpf_prog_free(struct bpf_prog_aux *aux) +static void selinux_bpf_prog_free(struct bpf_prog *prog) { - struct bpf_security_struct *bpfsec = aux->security; + struct bpf_security_struct *bpfsec = prog->aux->security; - aux->security = NULL; + prog->aux->security = NULL; kfree(bpfsec); } #endif @@ -7180,7 +7181,7 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = { LSM_HOOK_INIT(bpf_map, selinux_bpf_map), LSM_HOOK_INIT(bpf_prog, selinux_bpf_prog), LSM_HOOK_INIT(bpf_map_free_security, selinux_bpf_map_free), - LSM_HOOK_INIT(bpf_prog_free_security, selinux_bpf_prog_free), + LSM_HOOK_INIT(bpf_prog_free, selinux_bpf_prog_free), #endif #ifdef CONFIG_PERF_EVENTS @@ -7238,7 +7239,7 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = { #endif #ifdef CONFIG_BPF_SYSCALL LSM_HOOK_INIT(bpf_map_alloc_security, selinux_bpf_map_alloc), - LSM_HOOK_INIT(bpf_prog_alloc_security, selinux_bpf_prog_alloc), + LSM_HOOK_INIT(bpf_prog_load, selinux_bpf_prog_load), #endif #ifdef CONFIG_PERF_EVENTS LSM_HOOK_INIT(perf_event_alloc, selinux_perf_event_alloc), -- cgit From 66d636d70a79c1d37e3eea67ab50969e6aaef983 Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Thu, 30 Nov 2023 10:52:22 -0800 Subject: bpf,lsm: refactor bpf_map_alloc/bpf_map_free LSM hooks Similarly to bpf_prog_alloc LSM hook, rename and extend bpf_map_alloc hook into bpf_map_create, taking not just struct bpf_map, but also bpf_attr and bpf_token, to give a fuller context to LSMs. Unlike bpf_prog_alloc, there is no need to move the hook around, as it currently is firing right before allocating BPF map ID and FD, which seems to be a sweet spot. But like bpf_prog_alloc/bpf_prog_free combo, make sure that bpf_map_free LSM hook is called even if bpf_map_create hook returned error, as if few LSMs are combined together it could be that one LSM successfully allocated security blob for its needs, while subsequent LSM rejected BPF map creation. The former LSM would still need to free up LSM blob, so we need to ensure security_bpf_map_free() is called regardless of the outcome. Acked-by: Paul Moore Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/r/20231130185229.2688956-11-andrii@kernel.org Signed-off-by: Alexei Starovoitov --- include/linux/lsm_hook_defs.h | 5 +++-- include/linux/security.h | 6 ++++-- kernel/bpf/bpf_lsm.c | 6 +++--- kernel/bpf/syscall.c | 4 ++-- security/security.c | 16 ++++++++++------ security/selinux/hooks.c | 7 ++++--- 6 files changed, 26 insertions(+), 18 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 41ec4a7c070e..adb25cc63ce3 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -398,8 +398,9 @@ LSM_HOOK(void, LSM_RET_VOID, audit_rule_free, void *lsmrule) LSM_HOOK(int, 0, bpf, int cmd, union bpf_attr *attr, unsigned int size) LSM_HOOK(int, 0, bpf_map, struct bpf_map *map, fmode_t fmode) LSM_HOOK(int, 0, bpf_prog, struct bpf_prog *prog) -LSM_HOOK(int, 0, bpf_map_alloc_security, struct bpf_map *map) -LSM_HOOK(void, LSM_RET_VOID, bpf_map_free_security, struct bpf_map *map) +LSM_HOOK(int, 0, bpf_map_create, struct bpf_map *map, union bpf_attr *attr, + struct bpf_token *token) +LSM_HOOK(void, LSM_RET_VOID, bpf_map_free, struct bpf_map *map) LSM_HOOK(int, 0, bpf_prog_load, struct bpf_prog *prog, union bpf_attr *attr, struct bpf_token *token) LSM_HOOK(void, LSM_RET_VOID, bpf_prog_free, struct bpf_prog *prog) diff --git a/include/linux/security.h b/include/linux/security.h index 65467eef6678..08fd777cbe94 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -2025,7 +2025,8 @@ struct bpf_token; extern int security_bpf(int cmd, union bpf_attr *attr, unsigned int size); extern int security_bpf_map(struct bpf_map *map, fmode_t fmode); extern int security_bpf_prog(struct bpf_prog *prog); -extern int security_bpf_map_alloc(struct bpf_map *map); +extern int security_bpf_map_create(struct bpf_map *map, union bpf_attr *attr, + struct bpf_token *token); extern void security_bpf_map_free(struct bpf_map *map); extern int security_bpf_prog_load(struct bpf_prog *prog, union bpf_attr *attr, struct bpf_token *token); @@ -2047,7 +2048,8 @@ static inline int security_bpf_prog(struct bpf_prog *prog) return 0; } -static inline int security_bpf_map_alloc(struct bpf_map *map) +static inline int security_bpf_map_create(struct bpf_map *map, union bpf_attr *attr, + struct bpf_token *token) { return 0; } diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c index 3e956f6302f3..9e4e615f11eb 100644 --- a/kernel/bpf/bpf_lsm.c +++ b/kernel/bpf/bpf_lsm.c @@ -260,8 +260,8 @@ bpf_lsm_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) BTF_SET_START(sleepable_lsm_hooks) BTF_ID(func, bpf_lsm_bpf) BTF_ID(func, bpf_lsm_bpf_map) -BTF_ID(func, bpf_lsm_bpf_map_alloc_security) -BTF_ID(func, bpf_lsm_bpf_map_free_security) +BTF_ID(func, bpf_lsm_bpf_map_create) +BTF_ID(func, bpf_lsm_bpf_map_free) BTF_ID(func, bpf_lsm_bpf_prog) BTF_ID(func, bpf_lsm_bpf_prog_load) BTF_ID(func, bpf_lsm_bpf_prog_free) @@ -347,7 +347,7 @@ BTF_ID(func, bpf_lsm_userns_create) BTF_SET_END(sleepable_lsm_hooks) BTF_SET_START(untrusted_lsm_hooks) -BTF_ID(func, bpf_lsm_bpf_map_free_security) +BTF_ID(func, bpf_lsm_bpf_map_free) BTF_ID(func, bpf_lsm_bpf_prog_free) BTF_ID(func, bpf_lsm_file_alloc_security) BTF_ID(func, bpf_lsm_file_free_security) diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 7717c7c7b95d..aff045eed375 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -1309,9 +1309,9 @@ static int map_create(union bpf_attr *attr) attr->btf_vmlinux_value_type_id; } - err = security_bpf_map_alloc(map); + err = security_bpf_map_create(map, attr, token); if (err) - goto free_map; + goto free_map_sec; err = bpf_map_alloc_id(map); if (err) diff --git a/security/security.c b/security/security.c index c8a1c66cfaad..ad24cf36da94 100644 --- a/security/security.c +++ b/security/security.c @@ -5167,16 +5167,20 @@ int security_bpf_prog(struct bpf_prog *prog) } /** - * security_bpf_map_alloc() - Allocate a bpf map LSM blob - * @map: bpf map + * security_bpf_map_create() - Check if BPF map creation is allowed + * @map: BPF map object + * @attr: BPF syscall attributes used to create BPF map + * @token: BPF token used to grant user access * - * Initialize the security field inside bpf map. + * Do a check when the kernel creates a new BPF map. This is also the + * point where LSM blob is allocated for LSMs that need them. * * Return: Returns 0 on success, error on failure. */ -int security_bpf_map_alloc(struct bpf_map *map) +int security_bpf_map_create(struct bpf_map *map, union bpf_attr *attr, + struct bpf_token *token) { - return call_int_hook(bpf_map_alloc_security, 0, map); + return call_int_hook(bpf_map_create, 0, map, attr, token); } /** @@ -5205,7 +5209,7 @@ int security_bpf_prog_load(struct bpf_prog *prog, union bpf_attr *attr, */ void security_bpf_map_free(struct bpf_map *map) { - call_void_hook(bpf_map_free_security, map); + call_void_hook(bpf_map_free, map); } /** diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index eabee39e983c..002351ab67b7 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6783,7 +6783,8 @@ static int selinux_bpf_prog(struct bpf_prog *prog) BPF__PROG_RUN, NULL); } -static int selinux_bpf_map_alloc(struct bpf_map *map) +static int selinux_bpf_map_create(struct bpf_map *map, union bpf_attr *attr, + struct bpf_token *token) { struct bpf_security_struct *bpfsec; @@ -7180,7 +7181,7 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = { LSM_HOOK_INIT(bpf, selinux_bpf), LSM_HOOK_INIT(bpf_map, selinux_bpf_map), LSM_HOOK_INIT(bpf_prog, selinux_bpf_prog), - LSM_HOOK_INIT(bpf_map_free_security, selinux_bpf_map_free), + LSM_HOOK_INIT(bpf_map_free, selinux_bpf_map_free), LSM_HOOK_INIT(bpf_prog_free, selinux_bpf_prog_free), #endif @@ -7238,7 +7239,7 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = { LSM_HOOK_INIT(audit_rule_init, selinux_audit_rule_init), #endif #ifdef CONFIG_BPF_SYSCALL - LSM_HOOK_INIT(bpf_map_alloc_security, selinux_bpf_map_alloc), + LSM_HOOK_INIT(bpf_map_create, selinux_bpf_map_create), LSM_HOOK_INIT(bpf_prog_load, selinux_bpf_prog_load), #endif #ifdef CONFIG_PERF_EVENTS -- cgit From 36fb94944b35062db15ab3059f4123048cac658c Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Thu, 30 Nov 2023 10:52:29 -0800 Subject: bpf,selinux: allocate bpf_security_struct per BPF token Utilize newly added bpf_token_create/bpf_token_free LSM hooks to allocate struct bpf_security_struct for each BPF token object in SELinux. This just follows similar pattern for BPF prog and map. Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/r/20231130185229.2688956-18-andrii@kernel.org Signed-off-by: Alexei Starovoitov --- security/selinux/hooks.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 002351ab67b7..1501e95366a1 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6828,6 +6828,29 @@ static void selinux_bpf_prog_free(struct bpf_prog *prog) prog->aux->security = NULL; kfree(bpfsec); } + +static int selinux_bpf_token_create(struct bpf_token *token, union bpf_attr *attr, + struct path *path) +{ + struct bpf_security_struct *bpfsec; + + bpfsec = kzalloc(sizeof(*bpfsec), GFP_KERNEL); + if (!bpfsec) + return -ENOMEM; + + bpfsec->sid = current_sid(); + token->security = bpfsec; + + return 0; +} + +static void selinux_bpf_token_free(struct bpf_token *token) +{ + struct bpf_security_struct *bpfsec = token->security; + + token->security = NULL; + kfree(bpfsec); +} #endif struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = { @@ -7183,6 +7206,7 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = { LSM_HOOK_INIT(bpf_prog, selinux_bpf_prog), LSM_HOOK_INIT(bpf_map_free, selinux_bpf_map_free), LSM_HOOK_INIT(bpf_prog_free, selinux_bpf_prog_free), + LSM_HOOK_INIT(bpf_token_free, selinux_bpf_token_free), #endif #ifdef CONFIG_PERF_EVENTS @@ -7241,6 +7265,7 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = { #ifdef CONFIG_BPF_SYSCALL LSM_HOOK_INIT(bpf_map_create, selinux_bpf_map_create), LSM_HOOK_INIT(bpf_prog_load, selinux_bpf_prog_load), + LSM_HOOK_INIT(bpf_token_create, selinux_bpf_token_create), #endif #ifdef CONFIG_PERF_EVENTS LSM_HOOK_INIT(perf_event_alloc, selinux_perf_event_alloc), -- cgit