From 1b67772e4e3f16cd647b229cae95fc06d120be08 Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Tue, 23 Jan 2024 18:21:06 -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/ Signed-off-by: Andrii Nakryiko Signed-off-by: Alexei Starovoitov Acked-by: Paul Moore Link: https://lore.kernel.org/bpf/20240124022127.2379740-10-andrii@kernel.org --- 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 a6bf90ace84c..6d64fb189b1b 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6942,7 +6942,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; @@ -6951,16 +6952,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 @@ -7325,7 +7326,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 @@ -7383,7 +7384,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 v1.2.3 From a2431c7eabcf9bd5a1e7a1f7ecded40fdda4a8c5 Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Tue, 23 Jan 2024 18:21:07 -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. Signed-off-by: Andrii Nakryiko Signed-off-by: Alexei Starovoitov Acked-by: Paul Moore Link: https://lore.kernel.org/bpf/20240124022127.2379740-11-andrii@kernel.org --- 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 370181aa685b..1be4d3ca6efb 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -404,8 +404,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 cb2932fce448..83fcdc974116 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -2069,7 +2069,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); @@ -2091,7 +2092,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 7ee0dd011de4..76976908b302 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) @@ -359,7 +359,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 3857a2a9ae7f..b2750b79ac80 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -1318,9 +1318,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 eb159da4b146..26fcab35b6cd 100644 --- a/security/security.c +++ b/security/security.c @@ -5410,16 +5410,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); } /** @@ -5448,7 +5452,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 6d64fb189b1b..3d336a7952f7 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6920,7 +6920,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; @@ -7325,7 +7326,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 @@ -7383,7 +7384,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 v1.2.3 From 0054493e5141b16e316b8c52d6aa534397e48b6c Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Tue, 23 Jan 2024 18:21:14 -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 Signed-off-by: Alexei Starovoitov Link: https://lore.kernel.org/bpf/20240124022127.2379740-18-andrii@kernel.org --- 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 3d336a7952f7..8dd506ab9b1f 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6965,6 +6965,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 = { @@ -7328,6 +7351,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 @@ -7386,6 +7410,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 v1.2.3 From 90593caf7db74da2300f7a7056a26ae000b3e7cd Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Thu, 18 Jan 2024 20:44:21 -0500 Subject: selinux: reduce the object class calculations at inode init time We only need to call inode_mode_to_security_class() once in selinux_inode_init_security(). Reviewed-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/hooks.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index a6bf90ace84c..48ae90327fa4 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2920,23 +2920,22 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, struct superblock_security_struct *sbsec; struct xattr *xattr = lsm_get_xattr_slot(xattrs, xattr_count); u32 newsid, clen; + u16 newsclass; int rc; char *context; sbsec = selinux_superblock(dir->i_sb); newsid = tsec->create_sid; - - rc = selinux_determine_inode_label(tsec, dir, qstr, - inode_mode_to_security_class(inode->i_mode), - &newsid); + newsclass = inode_mode_to_security_class(inode->i_mode); + rc = selinux_determine_inode_label(tsec, dir, qstr, newsclass, &newsid); if (rc) return rc; /* Possibly defer initialization to selinux_complete_init. */ if (sbsec->flags & SE_SBINITIALIZED) { struct inode_security_struct *isec = selinux_inode(inode); - isec->sclass = inode_mode_to_security_class(inode->i_mode); + isec->sclass = newsclass; isec->sid = newsid; isec->initialized = LABEL_INITIALIZED; } -- cgit v1.2.3 From bfda63fa2243f66e3d07cce2f38cf12d68a2ddf0 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 30 Jan 2024 16:16:29 -0500 Subject: selinux: correct return values in selinux_socket_getpeersec_dgram() Instead of returning -EINVAL if any type of error occurs, limit -EINVAL to only those errors caused by passing a bad/invalid socket or packet/skb. In other cases where everything is correct but there isn't a valid peer label we return -ENOPROTOOPT. This helps make selinux_socket_getpeersec_dgram() more consistent with selinux_socket_getpeersec_stream(). Signed-off-by: Paul Moore --- security/selinux/hooks.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 48ae90327fa4..630ada3d208c 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -5193,11 +5193,11 @@ out_len: return err; } -static int selinux_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid) +static int selinux_socket_getpeersec_dgram(struct socket *sock, + struct sk_buff *skb, u32 *secid) { u32 peer_secid = SECSID_NULL; u16 family; - struct inode_security_struct *isec; if (skb && skb->protocol == htons(ETH_P_IP)) family = PF_INET; @@ -5205,19 +5205,21 @@ static int selinux_socket_getpeersec_dgram(struct socket *sock, struct sk_buff * family = PF_INET6; else if (sock) family = sock->sk->sk_family; - else - goto out; + else { + *secid = SECSID_NULL; + return -EINVAL; + } if (sock && family == PF_UNIX) { + struct inode_security_struct *isec; isec = inode_security_novalidate(SOCK_INODE(sock)); peer_secid = isec->sid; } else if (skb) selinux_skb_peerlbl_sid(skb, family, &peer_secid); -out: *secid = peer_secid; if (peer_secid == SECSID_NULL) - return -EINVAL; + return -ENOPROTOOPT; return 0; } -- cgit v1.2.3 From 7c655bee5cd8e060983bd89460fffc1f9f780cda Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Fri, 2 Feb 2024 17:40:48 +1100 Subject: selinux: only filter copy-up xattrs following initialization Extended attribute copy-up functionality added via 19472b69d639d ("selinux: Implementation for inode_copy_up_xattr() hook") sees "security.selinux" contexts dropped, instead relying on contexts applied via the inode_copy_up() hook. When copy-up takes place during early boot, prior to selinux initialization / policy load, the context stripping can be unwanted and unexpected. With this change, filtering of "security.selinux" xattrs will only occur after selinux initialization. Signed-off-by: David Disseldorp Signed-off-by: Paul Moore --- security/selinux/hooks.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 630ada3d208c..a0fde0641f77 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3533,9 +3533,10 @@ static int selinux_inode_copy_up_xattr(const char *name) { /* The copy_up hook above sets the initial context on an inode, but we * don't then want to overwrite it by blindly copying all the lower - * xattrs up. Instead, we have to filter out SELinux-related xattrs. + * xattrs up. Instead, filter out SELinux-related xattrs following + * policy load. */ - if (strcmp(name, XATTR_NAME_SELINUX) == 0) + if (selinux_initialized() && strcmp(name, XATTR_NAME_SELINUX) == 0) return 1; /* Discard */ /* * Any other attribute apart from SELINUX is not claimed, supported -- cgit v1.2.3 From 314a8dc728d038378795236f6b5199265f921f45 Mon Sep 17 00:00:00 2001 From: Roberto Sassu Date: Thu, 15 Feb 2024 11:30:57 +0100 Subject: security: Align inode_setattr hook definition with EVM Add the idmap parameter to the definition, so that evm_inode_setattr() can be registered as this hook implementation. Signed-off-by: Roberto Sassu Reviewed-by: Stefan Berger Acked-by: Casey Schaufler Reviewed-by: Mimi Zohar Signed-off-by: Paul Moore --- include/linux/lsm_hook_defs.h | 3 ++- security/security.c | 2 +- security/selinux/hooks.c | 3 ++- security/smack/smack_lsm.c | 4 +++- 4 files changed, 8 insertions(+), 4 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 76458b6d53da..b00b16d58413 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -135,7 +135,8 @@ LSM_HOOK(int, 0, inode_readlink, struct dentry *dentry) LSM_HOOK(int, 0, inode_follow_link, struct dentry *dentry, struct inode *inode, bool rcu) LSM_HOOK(int, 0, inode_permission, struct inode *inode, int mask) -LSM_HOOK(int, 0, inode_setattr, struct dentry *dentry, struct iattr *attr) +LSM_HOOK(int, 0, inode_setattr, struct mnt_idmap *idmap, struct dentry *dentry, + struct iattr *attr) LSM_HOOK(int, 0, inode_getattr, const struct path *path) LSM_HOOK(int, 0, inode_setxattr, struct mnt_idmap *idmap, struct dentry *dentry, const char *name, const void *value, diff --git a/security/security.c b/security/security.c index dc188201a4f5..9dc601d45960 100644 --- a/security/security.c +++ b/security/security.c @@ -2215,7 +2215,7 @@ int security_inode_setattr(struct mnt_idmap *idmap, if (unlikely(IS_PRIVATE(d_backing_inode(dentry)))) return 0; - ret = call_int_hook(inode_setattr, 0, dentry, attr); + ret = call_int_hook(inode_setattr, 0, idmap, dentry, attr); if (ret) return ret; return evm_inode_setattr(idmap, dentry, attr); diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index a6bf90ace84c..cedb4cbf072e 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3136,7 +3136,8 @@ static int selinux_inode_permission(struct inode *inode, int mask) return rc; } -static int selinux_inode_setattr(struct dentry *dentry, struct iattr *iattr) +static int selinux_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry, + struct iattr *iattr) { const struct cred *cred = current_cred(); struct inode *inode = d_backing_inode(dentry); diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 0fdbf04cc258..1b6abfdf7173 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -1233,12 +1233,14 @@ static int smack_inode_permission(struct inode *inode, int mask) /** * smack_inode_setattr - Smack check for setting attributes + * @idmap: idmap of the mount * @dentry: the object * @iattr: for the force flag * * Returns 0 if access is permitted, an error code otherwise */ -static int smack_inode_setattr(struct dentry *dentry, struct iattr *iattr) +static int smack_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry, + struct iattr *iattr) { struct smk_audit_info ad; int rc; -- cgit v1.2.3 From 86dc9693145bc3b2c21d2bc6a2563376ba8b15ff Mon Sep 17 00:00:00 2001 From: Mickaël Salaün Date: Fri, 23 Feb 2024 20:05:45 +0100 Subject: selinux: fix lsm_get_self_attr() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit selinux_getselfattr() doesn't properly initialize the string pointer it passes to selinux_lsm_getattr() which can cause a problem when an attribute hasn't been explicitly set; selinux_lsm_getattr() returns 0/success, but does not set or initialize the string label/attribute. Failure to properly initialize the string causes problems later in selinux_getselfattr() when the function attempts to kfree() the string. Cc: Casey Schaufler Fixes: 762c934317e6 ("SELinux: Add selfattr hooks") Suggested-by: Paul Moore [PM: description changes as discussed in the thread] Signed-off-by: Mickaël Salaün Signed-off-by: Paul Moore --- security/selinux/hooks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index a6bf90ace84c..338b023a8c3e 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6559,7 +6559,7 @@ static int selinux_getselfattr(unsigned int attr, struct lsm_ctx __user *ctx, size_t *size, u32 flags) { int rc; - char *val; + char *val = NULL; int val_len; val_len = selinux_lsm_getattr(attr, current, &val); -- cgit v1.2.3 From a5a858f622a0aff5cdb5e271442cd01b2a01467f Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Thu, 14 Mar 2024 11:31:26 -0400 Subject: lsm: use 32-bit compatible data types in LSM syscalls Change the size parameters in lsm_list_modules(), lsm_set_self_attr() and lsm_get_self_attr() from size_t to u32. This avoids the need to have different interfaces for 32 and 64 bit systems. Cc: stable@vger.kernel.org Fixes: a04a1198088a ("LSM: syscalls for current process attributes") Fixes: ad4aff9ec25f ("LSM: Create lsm_list_modules system call") Signed-off-by: Casey Schaufler Reported-and-reviewed-by: Dmitry V. Levin [PM: subject and metadata tweaks, syscall.h fixes] Signed-off-by: Paul Moore --- include/linux/lsm_hook_defs.h | 4 ++-- include/linux/security.h | 8 ++++---- include/linux/syscalls.h | 6 +++--- security/apparmor/lsm.c | 4 ++-- security/lsm_syscalls.c | 10 +++++----- security/security.c | 12 ++++++------ security/selinux/hooks.c | 4 ++-- security/smack/smack_lsm.c | 4 ++-- tools/testing/selftests/lsm/common.h | 6 +++--- tools/testing/selftests/lsm/lsm_get_self_attr_test.c | 10 +++++----- tools/testing/selftests/lsm/lsm_list_modules_test.c | 8 ++++---- tools/testing/selftests/lsm/lsm_set_self_attr_test.c | 6 +++--- 12 files changed, 41 insertions(+), 41 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index a8057a3f8de6..334e00efbde4 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -280,9 +280,9 @@ LSM_HOOK(int, 0, netlink_send, struct sock *sk, struct sk_buff *skb) LSM_HOOK(void, LSM_RET_VOID, d_instantiate, struct dentry *dentry, struct inode *inode) LSM_HOOK(int, -EOPNOTSUPP, getselfattr, unsigned int attr, - struct lsm_ctx __user *ctx, size_t *size, u32 flags) + struct lsm_ctx __user *ctx, u32 *size, u32 flags) LSM_HOOK(int, -EOPNOTSUPP, setselfattr, unsigned int attr, - struct lsm_ctx *ctx, size_t size, u32 flags) + struct lsm_ctx *ctx, u32 size, u32 flags) LSM_HOOK(int, -EINVAL, getprocattr, struct task_struct *p, const char *name, char **value) LSM_HOOK(int, -EINVAL, setprocattr, const char *name, void *value, size_t size) diff --git a/include/linux/security.h b/include/linux/security.h index f249f5b9a9d7..41a8f667bdfa 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -491,9 +491,9 @@ int security_sem_semop(struct kern_ipc_perm *sma, struct sembuf *sops, unsigned nsops, int alter); void security_d_instantiate(struct dentry *dentry, struct inode *inode); int security_getselfattr(unsigned int attr, struct lsm_ctx __user *ctx, - size_t __user *size, u32 flags); + u32 __user *size, u32 flags); int security_setselfattr(unsigned int attr, struct lsm_ctx __user *ctx, - size_t size, u32 flags); + u32 size, u32 flags); int security_getprocattr(struct task_struct *p, int lsmid, const char *name, char **value); int security_setprocattr(int lsmid, const char *name, void *value, size_t size); @@ -507,7 +507,7 @@ int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen); int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen); int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen); int security_locked_down(enum lockdown_reason what); -int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, size_t *uctx_len, +int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, u32 *uctx_len, void *val, size_t val_len, u64 id, u64 flags); #else /* CONFIG_SECURITY */ @@ -1478,7 +1478,7 @@ static inline int security_locked_down(enum lockdown_reason what) return 0; } static inline int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, - size_t *uctx_len, void *val, size_t val_len, + u32 *uctx_len, void *val, size_t val_len, u64 id, u64 flags) { return -EOPNOTSUPP; diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h index 77eb9b0e7685..e619ac10cd23 100644 --- a/include/linux/syscalls.h +++ b/include/linux/syscalls.h @@ -960,10 +960,10 @@ asmlinkage long sys_cachestat(unsigned int fd, struct cachestat __user *cstat, unsigned int flags); asmlinkage long sys_map_shadow_stack(unsigned long addr, unsigned long size, unsigned int flags); asmlinkage long sys_lsm_get_self_attr(unsigned int attr, struct lsm_ctx *ctx, - size_t *size, __u32 flags); + u32 *size, u32 flags); asmlinkage long sys_lsm_set_self_attr(unsigned int attr, struct lsm_ctx *ctx, - size_t size, __u32 flags); -asmlinkage long sys_lsm_list_modules(u64 *ids, size_t *size, u32 flags); + u32 size, u32 flags); +asmlinkage long sys_lsm_list_modules(u64 *ids, u32 *size, u32 flags); /* * Architecture-specific system calls diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 9a3dcaafb5b1..cef8c466af80 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -779,7 +779,7 @@ static int apparmor_sb_pivotroot(const struct path *old_path, } static int apparmor_getselfattr(unsigned int attr, struct lsm_ctx __user *lx, - size_t *size, u32 flags) + u32 *size, u32 flags) { int error = -ENOENT; struct aa_task_ctx *ctx = task_ctx(current); @@ -924,7 +924,7 @@ fail: } static int apparmor_setselfattr(unsigned int attr, struct lsm_ctx *ctx, - size_t size, u32 flags) + u32 size, u32 flags) { int rc; diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c index 5d391b1f7e69..8440948a690c 100644 --- a/security/lsm_syscalls.c +++ b/security/lsm_syscalls.c @@ -53,7 +53,7 @@ u64 lsm_name_to_attr(const char *name) * value indicating the reason for the error is returned. */ SYSCALL_DEFINE4(lsm_set_self_attr, unsigned int, attr, struct lsm_ctx __user *, - ctx, size_t, size, u32, flags) + ctx, u32, size, u32, flags) { return security_setselfattr(attr, ctx, size, flags); } @@ -75,7 +75,7 @@ SYSCALL_DEFINE4(lsm_set_self_attr, unsigned int, attr, struct lsm_ctx __user *, * a negative value indicating the error is returned. */ SYSCALL_DEFINE4(lsm_get_self_attr, unsigned int, attr, struct lsm_ctx __user *, - ctx, size_t __user *, size, u32, flags) + ctx, u32 __user *, size, u32, flags) { return security_getselfattr(attr, ctx, size, flags); } @@ -93,11 +93,11 @@ SYSCALL_DEFINE4(lsm_get_self_attr, unsigned int, attr, struct lsm_ctx __user *, * required size. In all other cases a negative value indicating the * error is returned. */ -SYSCALL_DEFINE3(lsm_list_modules, u64 __user *, ids, size_t __user *, size, +SYSCALL_DEFINE3(lsm_list_modules, u64 __user *, ids, u32 __user *, size, u32, flags) { - size_t total_size = lsm_active_cnt * sizeof(*ids); - size_t usize; + u32 total_size = lsm_active_cnt * sizeof(*ids); + u32 usize; int i; if (flags) diff --git a/security/security.c b/security/security.c index a07f43c58d65..5b2e0a15377d 100644 --- a/security/security.c +++ b/security/security.c @@ -785,7 +785,7 @@ static int lsm_superblock_alloc(struct super_block *sb) * Returns 0 on success, -E2BIG if userspace buffer is not large enough, * -EFAULT on a copyout error, -ENOMEM if memory can't be allocated. */ -int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, size_t *uctx_len, +int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, u32 *uctx_len, void *val, size_t val_len, u64 id, u64 flags) { @@ -3931,14 +3931,14 @@ EXPORT_SYMBOL(security_d_instantiate); * If @size is insufficient to contain the data -E2BIG is returned. */ int security_getselfattr(unsigned int attr, struct lsm_ctx __user *uctx, - size_t __user *size, u32 flags) + u32 __user *size, u32 flags) { struct security_hook_list *hp; struct lsm_ctx lctx = { .id = LSM_ID_UNDEF, }; u8 __user *base = (u8 __user *)uctx; - size_t total = 0; - size_t entrysize; - size_t left; + u32 entrysize; + u32 total = 0; + u32 left; bool toobig = false; bool single = false; int count = 0; @@ -4024,7 +4024,7 @@ int security_getselfattr(unsigned int attr, struct lsm_ctx __user *uctx, * LSM specific failure. */ int security_setselfattr(unsigned int attr, struct lsm_ctx __user *uctx, - size_t size, u32 flags) + u32 size, u32 flags) { struct security_hook_list *hp; struct lsm_ctx *lctx; diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 8db4875164ab..3448454c82d0 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6559,7 +6559,7 @@ abort_change: * There will only ever be one attribute. */ static int selinux_getselfattr(unsigned int attr, struct lsm_ctx __user *ctx, - size_t *size, u32 flags) + u32 *size, u32 flags) { int rc; char *val = NULL; @@ -6574,7 +6574,7 @@ static int selinux_getselfattr(unsigned int attr, struct lsm_ctx __user *ctx, } static int selinux_setselfattr(unsigned int attr, struct lsm_ctx *ctx, - size_t size, u32 flags) + u32 size, u32 flags) { int rc; diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 28be26712396..146667937811 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -3653,7 +3653,7 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode) * There will only ever be one attribute. */ static int smack_getselfattr(unsigned int attr, struct lsm_ctx __user *ctx, - size_t *size, u32 flags) + u32 *size, u32 flags) { int rc; struct smack_known *skp; @@ -3774,7 +3774,7 @@ static int do_setattr(u64 attr, void *value, size_t size) * Returns 0 on success, an error code otherwise. */ static int smack_setselfattr(unsigned int attr, struct lsm_ctx *ctx, - size_t size, u32 flags) + u32 size, u32 flags) { int rc; diff --git a/tools/testing/selftests/lsm/common.h b/tools/testing/selftests/lsm/common.h index d404329e5eeb..06d12110d241 100644 --- a/tools/testing/selftests/lsm/common.h +++ b/tools/testing/selftests/lsm/common.h @@ -7,7 +7,7 @@ #ifndef lsm_get_self_attr static inline int lsm_get_self_attr(unsigned int attr, struct lsm_ctx *ctx, - size_t *size, __u32 flags) + __u32 *size, __u32 flags) { return syscall(__NR_lsm_get_self_attr, attr, ctx, size, flags); } @@ -15,14 +15,14 @@ static inline int lsm_get_self_attr(unsigned int attr, struct lsm_ctx *ctx, #ifndef lsm_set_self_attr static inline int lsm_set_self_attr(unsigned int attr, struct lsm_ctx *ctx, - size_t size, __u32 flags) + __u32 size, __u32 flags) { return syscall(__NR_lsm_set_self_attr, attr, ctx, size, flags); } #endif #ifndef lsm_list_modules -static inline int lsm_list_modules(__u64 *ids, size_t *size, __u32 flags) +static inline int lsm_list_modules(__u64 *ids, __u32 *size, __u32 flags) { return syscall(__NR_lsm_list_modules, ids, size, flags); } diff --git a/tools/testing/selftests/lsm/lsm_get_self_attr_test.c b/tools/testing/selftests/lsm/lsm_get_self_attr_test.c index e0e313d9047a..df215e4aa63f 100644 --- a/tools/testing/selftests/lsm/lsm_get_self_attr_test.c +++ b/tools/testing/selftests/lsm/lsm_get_self_attr_test.c @@ -40,7 +40,7 @@ TEST(size_null_lsm_get_self_attr) TEST(ctx_null_lsm_get_self_attr) { const long page_size = sysconf(_SC_PAGESIZE); - size_t size = page_size; + __u32 size = page_size; int rc; rc = lsm_get_self_attr(LSM_ATTR_CURRENT, NULL, &size, 0); @@ -57,7 +57,7 @@ TEST(size_too_small_lsm_get_self_attr) { const long page_size = sysconf(_SC_PAGESIZE); struct lsm_ctx *ctx = calloc(page_size, 1); - size_t size = 1; + __u32 size = 1; ASSERT_NE(NULL, ctx); errno = 0; @@ -77,7 +77,7 @@ TEST(flags_zero_lsm_get_self_attr) const long page_size = sysconf(_SC_PAGESIZE); struct lsm_ctx *ctx = calloc(page_size, 1); __u64 *syscall_lsms = calloc(page_size, 1); - size_t size; + __u32 size; int lsmcount; int i; @@ -117,7 +117,7 @@ TEST(flags_overset_lsm_get_self_attr) { const long page_size = sysconf(_SC_PAGESIZE); struct lsm_ctx *ctx = calloc(page_size, 1); - size_t size; + __u32 size; ASSERT_NE(NULL, ctx); @@ -140,7 +140,7 @@ TEST(flags_overset_lsm_get_self_attr) TEST(basic_lsm_get_self_attr) { const long page_size = sysconf(_SC_PAGESIZE); - size_t size = page_size; + __u32 size = page_size; struct lsm_ctx *ctx = calloc(page_size, 1); struct lsm_ctx *tctx = NULL; __u64 *syscall_lsms = calloc(page_size, 1); diff --git a/tools/testing/selftests/lsm/lsm_list_modules_test.c b/tools/testing/selftests/lsm/lsm_list_modules_test.c index 4d5d4cee2586..06d24d4679a6 100644 --- a/tools/testing/selftests/lsm/lsm_list_modules_test.c +++ b/tools/testing/selftests/lsm/lsm_list_modules_test.c @@ -31,7 +31,7 @@ TEST(size_null_lsm_list_modules) TEST(ids_null_lsm_list_modules) { const long page_size = sysconf(_SC_PAGESIZE); - size_t size = page_size; + __u32 size = page_size; errno = 0; ASSERT_EQ(-1, lsm_list_modules(NULL, &size, 0)); @@ -43,7 +43,7 @@ TEST(size_too_small_lsm_list_modules) { const long page_size = sysconf(_SC_PAGESIZE); __u64 *syscall_lsms = calloc(page_size, 1); - size_t size = 1; + __u32 size = 1; ASSERT_NE(NULL, syscall_lsms); errno = 0; @@ -58,7 +58,7 @@ TEST(flags_set_lsm_list_modules) { const long page_size = sysconf(_SC_PAGESIZE); __u64 *syscall_lsms = calloc(page_size, 1); - size_t size = page_size; + __u32 size = page_size; ASSERT_NE(NULL, syscall_lsms); errno = 0; @@ -72,7 +72,7 @@ TEST(flags_set_lsm_list_modules) TEST(correct_lsm_list_modules) { const long page_size = sysconf(_SC_PAGESIZE); - size_t size = page_size; + __u32 size = page_size; __u64 *syscall_lsms = calloc(page_size, 1); char *sysfs_lsms = calloc(page_size, 1); char *name; diff --git a/tools/testing/selftests/lsm/lsm_set_self_attr_test.c b/tools/testing/selftests/lsm/lsm_set_self_attr_test.c index e9712c6cf596..66dec47e3ca3 100644 --- a/tools/testing/selftests/lsm/lsm_set_self_attr_test.c +++ b/tools/testing/selftests/lsm/lsm_set_self_attr_test.c @@ -25,7 +25,7 @@ TEST(size_too_small_lsm_set_self_attr) { const long page_size = sysconf(_SC_PAGESIZE); struct lsm_ctx *ctx = calloc(page_size, 1); - size_t size = page_size; + __u32 size = page_size; ASSERT_NE(NULL, ctx); if (attr_lsm_count()) { @@ -41,7 +41,7 @@ TEST(flags_zero_lsm_set_self_attr) { const long page_size = sysconf(_SC_PAGESIZE); struct lsm_ctx *ctx = calloc(page_size, 1); - size_t size = page_size; + __u32 size = page_size; ASSERT_NE(NULL, ctx); if (attr_lsm_count()) { @@ -57,7 +57,7 @@ TEST(flags_overset_lsm_set_self_attr) { const long page_size = sysconf(_SC_PAGESIZE); char *ctx = calloc(page_size, 1); - size_t size = page_size; + __u32 size = page_size; struct lsm_ctx *tctx = (struct lsm_ctx *)ctx; ASSERT_NE(NULL, ctx); -- cgit v1.2.3