From 6bcdfd2cac5559c680aef8dd4c5facada55ab623 Mon Sep 17 00:00:00 2001 From: Roberto Sassu Date: Sat, 10 Jun 2023 09:57:35 +0200 Subject: security: Allow all LSMs to provide xattrs for inode_init_security hook Currently, the LSM infrastructure supports only one LSM providing an xattr and EVM calculating the HMAC on that xattr, plus other inode metadata. Allow all LSMs to provide one or multiple xattrs, by extending the security blob reservation mechanism. Introduce the new lbs_xattr_count field of the lsm_blob_sizes structure, so that each LSM can specify how many xattrs it needs, and the LSM infrastructure knows how many xattr slots it should allocate. Modify the inode_init_security hook definition, by passing the full xattr array allocated in security_inode_init_security(), and the current number of xattr slots in that array filled by LSMs. The first parameter would allow EVM to access and calculate the HMAC on xattrs supplied by other LSMs, the second to not leave gaps in the xattr array, when an LSM requested but did not provide xattrs (e.g. if it is not initialized). Introduce lsm_get_xattr_slot(), which LSMs can call as many times as the number specified in the lbs_xattr_count field of the lsm_blob_sizes structure. During each call, lsm_get_xattr_slot() increments the number of filled xattrs, so that at the next invocation it returns the next xattr slot to fill. Cleanup security_inode_init_security(). Unify the !initxattrs and initxattrs case by simply not allocating the new_xattrs array in the former. Update the documentation to reflect the changes, and fix the description of the xattr name, as it is not allocated anymore. Adapt both SELinux and Smack to use the new definition of the inode_init_security hook, and to call lsm_get_xattr_slot() to obtain and fill the reserved slots in the xattr array. Move the xattr->name assignment after the xattr->value one, so that it is done only in case of successful memory allocation. Finally, change the default return value of the inode_init_security hook from zero to -EOPNOTSUPP, so that BPF LSM correctly follows the hook conventions. Reported-by: Nicolas Bouchinet Link: https://lore.kernel.org/linux-integrity/Y1FTSIo+1x+4X0LS@archlinux/ Signed-off-by: Roberto Sassu Acked-by: Casey Schaufler [PM: minor comment and variable tweaks, approved by RS] Signed-off-by: Paul Moore --- security/selinux/hooks.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index d06e350fedee..a0787f07d745 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -104,6 +104,8 @@ #include "audit.h" #include "avc_ss.h" +#define SELINUX_INODE_INIT_XATTRS 1 + struct selinux_state selinux_state; /* SECMARK reference count */ @@ -2847,11 +2849,11 @@ static int selinux_dentry_create_files_as(struct dentry *dentry, int mode, static int selinux_inode_init_security(struct inode *inode, struct inode *dir, const struct qstr *qstr, - const char **name, - void **value, size_t *len) + struct xattr *xattrs, int *xattr_count) { const struct task_security_struct *tsec = selinux_cred(current_cred()); struct superblock_security_struct *sbsec; + struct xattr *xattr = lsm_get_xattr_slot(xattrs, xattr_count); u32 newsid, clen; int rc; char *context; @@ -2878,16 +2880,14 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, !(sbsec->flags & SBLABEL_MNT)) return -EOPNOTSUPP; - if (name) - *name = XATTR_SELINUX_SUFFIX; - - if (value && len) { + if (xattr) { rc = security_sid_to_context_force(newsid, &context, &clen); if (rc) return rc; - *value = context; - *len = clen; + xattr->value = context; + xattr->value_len = clen; + xattr->name = XATTR_SELINUX_SUFFIX; } return 0; @@ -6815,6 +6815,7 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = { .lbs_ipc = sizeof(struct ipc_security_struct), .lbs_msg_msg = sizeof(struct msg_security_struct), .lbs_superblock = sizeof(struct superblock_security_struct), + .lbs_xattr_count = SELINUX_INODE_INIT_XATTRS, }; #ifdef CONFIG_PERF_EVENTS -- cgit From 6672efbb685f7c9c9df005beb839e1942fd6b34e Mon Sep 17 00:00:00 2001 From: Khadija Kamran Date: Mon, 7 Aug 2023 11:59:29 +0500 Subject: lsm: constify the 'target' parameter in security_capget() Three LSMs register the implementations for the "capget" hook: AppArmor, SELinux, and the normal capability code. Looking at the function implementations we may observe that the first parameter "target" is not changing. Mark the first argument "target" of LSM hook security_capget() as "const" since it will not be changing in the LSM hook. cap_capget() LSM hook declaration exceeds the 80 characters per line limit. Split the function declaration to multiple lines to decrease the line length. Signed-off-by: Khadija Kamran Acked-by: John Johansen [PM: align the cap_capget() declaration, spelling fixes] Signed-off-by: Paul Moore --- security/selinux/hooks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index a0787f07d745..c816dc5de627 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2082,7 +2082,7 @@ static int selinux_ptrace_traceme(struct task_struct *parent) SECCLASS_PROCESS, PROCESS__PTRACE, NULL); } -static int selinux_capget(struct task_struct *target, kernel_cap_t *effective, +static int selinux_capget(const struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted) { return avc_has_perm(current_sid(), task_sid_obj(target), -- cgit From 8e4672d6f902d5c4db1e87e8aa9f530149d85bc6 Mon Sep 17 00:00:00 2001 From: Khadija Kamran Date: Sat, 12 Aug 2023 20:31:08 +0500 Subject: lsm: constify the 'file' parameter in security_binder_transfer_file() SELinux registers the implementation for the "binder_transfer_file" hook. Looking at the function implementation we observe that the parameter "file" is not changing. Mark the "file" parameter of LSM hook security_binder_transfer_file() as "const" since it will not be changing in the LSM hook. Signed-off-by: Khadija Kamran [PM: subject line whitespace fix] Signed-off-by: Paul Moore --- security/selinux/hooks.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index c816dc5de627..ee7c49c2cfd3 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1691,7 +1691,7 @@ static inline int file_path_has_perm(const struct cred *cred, } #ifdef CONFIG_BPF_SYSCALL -static int bpf_fd_pass(struct file *file, u32 sid); +static int bpf_fd_pass(const struct file *file, u32 sid); #endif /* Check whether a task can use an open file descriptor to @@ -1952,7 +1952,7 @@ static inline u32 file_mask_to_av(int mode, int mask) } /* Convert a Linux file to an access vector. */ -static inline u32 file_to_av(struct file *file) +static inline u32 file_to_av(const struct file *file) { u32 av = 0; @@ -2027,7 +2027,7 @@ static int selinux_binder_transfer_binder(const struct cred *from, static int selinux_binder_transfer_file(const struct cred *from, const struct cred *to, - struct file *file) + const struct file *file) { u32 sid = cred_sid(to); struct file_security_struct *fsec = selinux_file(file); @@ -6718,7 +6718,7 @@ static u32 bpf_map_fmode_to_av(fmode_t fmode) * access the bpf object and that's why we have to add this additional check in * selinux_file_receive and selinux_binder_transfer_files. */ -static int bpf_fd_pass(struct file *file, u32 sid) +static int bpf_fd_pass(const struct file *file, u32 sid) { struct bpf_security_struct *bpfsec; struct bpf_prog *prog; -- cgit