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 v1.2.3-58-ga151 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 --- include/linux/lsm_hook_defs.h | 2 +- include/linux/security.h | 7 ++++--- kernel/capability.c | 2 +- security/apparmor/lsm.c | 2 +- security/commoncap.c | 2 +- security/security.c | 2 +- security/selinux/hooks.c | 2 +- 7 files changed, 10 insertions(+), 9 deletions(-) (limited to 'security/selinux') diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 920d8f16fa99..540caa703573 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -36,7 +36,7 @@ LSM_HOOK(int, 0, binder_transfer_file, const struct cred *from, LSM_HOOK(int, 0, ptrace_access_check, struct task_struct *child, unsigned int mode) LSM_HOOK(int, 0, ptrace_traceme, struct task_struct *parent) -LSM_HOOK(int, 0, capget, struct task_struct *target, kernel_cap_t *effective, +LSM_HOOK(int, 0, capget, const struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted) LSM_HOOK(int, 0, capset, struct cred *new, const struct cred *old, const kernel_cap_t *effective, const kernel_cap_t *inheritable, diff --git a/include/linux/security.h b/include/linux/security.h index 32828502f09e..7665f56d920a 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -145,7 +145,8 @@ extern int cap_capable(const struct cred *cred, struct user_namespace *ns, extern int cap_settime(const struct timespec64 *ts, const struct timezone *tz); extern int cap_ptrace_access_check(struct task_struct *child, unsigned int mode); extern int cap_ptrace_traceme(struct task_struct *parent); -extern int cap_capget(struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted); +extern int cap_capget(const struct task_struct *target, kernel_cap_t *effective, + kernel_cap_t *inheritable, kernel_cap_t *permitted); extern int cap_capset(struct cred *new, const struct cred *old, const kernel_cap_t *effective, const kernel_cap_t *inheritable, @@ -271,7 +272,7 @@ int security_binder_transfer_file(const struct cred *from, const struct cred *to, struct file *file); int security_ptrace_access_check(struct task_struct *child, unsigned int mode); int security_ptrace_traceme(struct task_struct *parent); -int security_capget(struct task_struct *target, +int security_capget(const struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted); @@ -553,7 +554,7 @@ static inline int security_ptrace_traceme(struct task_struct *parent) return cap_ptrace_traceme(parent); } -static inline int security_capget(struct task_struct *target, +static inline int security_capget(const struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted) diff --git a/kernel/capability.c b/kernel/capability.c index 1a2795102ae4..dac4df77e376 100644 --- a/kernel/capability.c +++ b/kernel/capability.c @@ -112,7 +112,7 @@ static inline int cap_get_target_pid(pid_t pid, kernel_cap_t *pEp, int ret; if (pid && (pid != task_pid_vnr(current))) { - struct task_struct *target; + const struct task_struct *target; rcu_read_lock(); diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index c9463bd0307d..108eccc5ada5 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -144,7 +144,7 @@ static int apparmor_ptrace_traceme(struct task_struct *parent) } /* Derived from security/commoncap.c:cap_capget */ -static int apparmor_capget(struct task_struct *target, kernel_cap_t *effective, +static int apparmor_capget(const struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted) { struct aa_label *label; diff --git a/security/commoncap.c b/security/commoncap.c index ab5742ab4362..bc0521104197 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -197,7 +197,7 @@ out: * This function retrieves the capabilities of the nominated task and returns * them to the caller. */ -int cap_capget(struct task_struct *target, kernel_cap_t *effective, +int cap_capget(const struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted) { const struct cred *cred; diff --git a/security/security.c b/security/security.c index 9177fd0968bd..6962ea38a98f 100644 --- a/security/security.c +++ b/security/security.c @@ -894,7 +894,7 @@ int security_ptrace_traceme(struct task_struct *parent) * * Return: Returns 0 if the capability sets were successfully obtained. */ -int security_capget(struct task_struct *target, +int security_capget(const struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted) 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 v1.2.3-58-ga151 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 --- include/linux/lsm_hook_defs.h | 2 +- include/linux/security.h | 4 ++-- security/security.c | 2 +- security/selinux/hooks.c | 8 ++++---- 4 files changed, 8 insertions(+), 8 deletions(-) (limited to 'security/selinux') diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 540caa703573..4bdddb52a8fe 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -32,7 +32,7 @@ LSM_HOOK(int, 0, binder_transaction, const struct cred *from, LSM_HOOK(int, 0, binder_transfer_binder, const struct cred *from, const struct cred *to) LSM_HOOK(int, 0, binder_transfer_file, const struct cred *from, - const struct cred *to, struct file *file) + const struct cred *to, const struct file *file) LSM_HOOK(int, 0, ptrace_access_check, struct task_struct *child, unsigned int mode) LSM_HOOK(int, 0, ptrace_traceme, struct task_struct *parent) diff --git a/include/linux/security.h b/include/linux/security.h index 7665f56d920a..dcb3604ffab8 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -269,7 +269,7 @@ int security_binder_transaction(const struct cred *from, int security_binder_transfer_binder(const struct cred *from, const struct cred *to); int security_binder_transfer_file(const struct cred *from, - const struct cred *to, struct file *file); + const struct cred *to, const struct file *file); int security_ptrace_access_check(struct task_struct *child, unsigned int mode); int security_ptrace_traceme(struct task_struct *parent); int security_capget(const struct task_struct *target, @@ -538,7 +538,7 @@ static inline int security_binder_transfer_binder(const struct cred *from, static inline int security_binder_transfer_file(const struct cred *from, const struct cred *to, - struct file *file) + const struct file *file) { return 0; } diff --git a/security/security.c b/security/security.c index 6962ea38a98f..96f2c68a1571 100644 --- a/security/security.c +++ b/security/security.c @@ -841,7 +841,7 @@ int security_binder_transfer_binder(const struct cred *from, * Return: Returns 0 if permission is granted. */ int security_binder_transfer_file(const struct cred *from, - const struct cred *to, struct file *file) + const struct cred *to, const struct file *file) { return call_int_hook(binder_transfer_file, 0, from, to, file); } 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 v1.2.3-58-ga151