diff options
author | Linus Torvalds <torvalds@linux-foundation.org> | 2022-12-13 09:47:48 -0800 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2022-12-13 09:47:48 -0800 |
commit | c76ff350bd57682ae12bea6383dd8baf4824ac96 (patch) | |
tree | 35b5e26c81e0ea1bed8c68696c5878c353923cb6 /security | |
parent | 57888f7b952d3f2696f82a701f1b3d9de7e346d3 (diff) | |
parent | 577cc1434e4cc1342c3df6d6a3c85136ab335c81 (diff) |
Merge tag 'lsm-pr-20221212' of git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm
Pull lsm updates from Paul Moore:
- Improve the error handling in the device cgroup such that memory
allocation failures when updating the access policy do not
potentially alter the policy.
- Some minor fixes to reiserfs to ensure that it properly releases
LSM-related xattr values.
- Update the security_socket_getpeersec_stream() LSM hook to take
sockptr_t values.
Previously the net/BPF folks updated the getsockopt code in the
network stack to leverage the sockptr_t type to make it easier to
pass both kernel and __user pointers, but unfortunately when they did
so they didn't convert the LSM hook.
While there was/is no immediate risk by not converting the LSM hook,
it seems like this is a mistake waiting to happen so this patch
proactively does the LSM hook conversion.
- Convert vfs_getxattr_alloc() to return an int instead of a ssize_t
and cleanup the callers. Internally the function was never going to
return anything larger than an int and the callers were doing some
very odd things casting the return value; this patch fixes all that
and helps bring a bit of sanity to vfs_getxattr_alloc() and its
callers.
- More verbose, and helpful, LSM debug output when the system is booted
with "lsm.debug" on the command line. There are examples in the
commit description, but the quick summary is that this patch provides
better information about which LSMs are enabled and the ordering in
which they are processed.
- General comment and kernel-doc fixes and cleanups.
* tag 'lsm-pr-20221212' of git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm:
lsm: Fix description of fs_context_parse_param
lsm: Add/fix return values in lsm_hooks.h and fix formatting
lsm: Clarify documentation of vm_enough_memory hook
reiserfs: Add missing calls to reiserfs_security_free()
lsm,fs: fix vfs_getxattr_alloc() return type and caller error paths
device_cgroup: Roll back to original exceptions after copy failure
LSM: Better reporting of actual LSMs at boot
lsm: make security_socket_getpeersec_stream() sockptr_t safe
audit: Fix some kernel-doc warnings
lsm: remove obsoleted comments for security hooks
fs: edit a comment made in bad taste
Diffstat (limited to 'security')
-rw-r--r-- | security/apparmor/domain.c | 3 | ||||
-rw-r--r-- | security/apparmor/lsm.c | 29 | ||||
-rw-r--r-- | security/commoncap.c | 22 | ||||
-rw-r--r-- | security/device_cgroup.c | 33 | ||||
-rw-r--r-- | security/integrity/evm/evm_crypto.c | 5 | ||||
-rw-r--r-- | security/integrity/evm/evm_main.c | 7 | ||||
-rw-r--r-- | security/integrity/ima/ima.h | 5 | ||||
-rw-r--r-- | security/integrity/ima/ima_appraise.c | 6 | ||||
-rw-r--r-- | security/integrity/ima/ima_main.c | 6 | ||||
-rw-r--r-- | security/integrity/ima/ima_template_lib.c | 11 | ||||
-rw-r--r-- | security/lsm_audit.c | 1 | ||||
-rw-r--r-- | security/security.c | 51 | ||||
-rw-r--r-- | security/selinux/hooks.c | 13 | ||||
-rw-r--r-- | security/smack/smack_lsm.c | 19 |
14 files changed, 131 insertions, 80 deletions
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 7bafb4c4767c..00dc0ec066de 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -311,10 +311,9 @@ static int aa_xattrs_match(const struct linux_binprm *bprm, struct aa_profile *profile, unsigned int state) { int i; - ssize_t size; struct dentry *d; char *value = NULL; - int value_size = 0, ret = profile->xattr_count; + int size, value_size = 0, ret = profile->xattr_count; if (!bprm || !profile->xattr_count) return 0; diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index b751d6253977..ff14fe0ffca2 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -1117,11 +1117,10 @@ static struct aa_label *sk_peer_label(struct sock *sk) * Note: for tcp only valid if using ipsec or cipso on lan */ static int apparmor_socket_getpeersec_stream(struct socket *sock, - char __user *optval, - int __user *optlen, + sockptr_t optval, sockptr_t optlen, unsigned int len) { - char *name; + char *name = NULL; int slen, error = 0; struct aa_label *label; struct aa_label *peer; @@ -1138,23 +1137,21 @@ static int apparmor_socket_getpeersec_stream(struct socket *sock, /* don't include terminating \0 in slen, it breaks some apps */ if (slen < 0) { error = -ENOMEM; - } else { - if (slen > len) { - error = -ERANGE; - } else if (copy_to_user(optval, name, slen)) { - error = -EFAULT; - goto out; - } - if (put_user(slen, optlen)) - error = -EFAULT; -out: - kfree(name); - + goto done; + } + if (slen > len) { + error = -ERANGE; + goto done_len; } + if (copy_to_sockptr(optval, name, slen)) + error = -EFAULT; +done_len: + if (copy_to_sockptr(optlen, &slen, sizeof(slen))) + error = -EFAULT; done: end_current_label_crit_section(label); - + kfree(name); return error; } diff --git a/security/commoncap.c b/security/commoncap.c index 5d9570f54a1c..1164278b97fd 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -352,14 +352,14 @@ static __u32 sansflags(__u32 m) return m & ~VFS_CAP_FLAGS_EFFECTIVE; } -static bool is_v2header(size_t size, const struct vfs_cap_data *cap) +static bool is_v2header(int size, const struct vfs_cap_data *cap) { if (size != XATTR_CAPS_SZ_2) return false; return sansflags(le32_to_cpu(cap->magic_etc)) == VFS_CAP_REVISION_2; } -static bool is_v3header(size_t size, const struct vfs_cap_data *cap) +static bool is_v3header(int size, const struct vfs_cap_data *cap) { if (size != XATTR_CAPS_SZ_3) return false; @@ -381,7 +381,7 @@ int cap_inode_getsecurity(struct user_namespace *mnt_userns, struct inode *inode, const char *name, void **buffer, bool alloc) { - int size, ret; + int size; kuid_t kroot; vfsuid_t vfsroot; u32 nsmagic, magic; @@ -398,22 +398,18 @@ int cap_inode_getsecurity(struct user_namespace *mnt_userns, dentry = d_find_any_alias(inode); if (!dentry) return -EINVAL; - - size = sizeof(struct vfs_ns_cap_data); - ret = (int)vfs_getxattr_alloc(mnt_userns, dentry, XATTR_NAME_CAPS, - &tmpbuf, size, GFP_NOFS); + size = vfs_getxattr_alloc(mnt_userns, dentry, XATTR_NAME_CAPS, &tmpbuf, + sizeof(struct vfs_ns_cap_data), GFP_NOFS); dput(dentry); - - if (ret < 0 || !tmpbuf) { - size = ret; + /* gcc11 complains if we don't check for !tmpbuf */ + if (size < 0 || !tmpbuf) goto out_free; - } fs_ns = inode->i_sb->s_user_ns; cap = (struct vfs_cap_data *) tmpbuf; - if (is_v2header((size_t) ret, cap)) { + if (is_v2header(size, cap)) { root = 0; - } else if (is_v3header((size_t) ret, cap)) { + } else if (is_v3header(size, cap)) { nscap = (struct vfs_ns_cap_data *) tmpbuf; root = le32_to_cpu(nscap->rootid); } else { diff --git a/security/device_cgroup.c b/security/device_cgroup.c index a9f8c63a96d1..bef2b9285fb3 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -82,6 +82,17 @@ free_and_exit: return -ENOMEM; } +static void dev_exceptions_move(struct list_head *dest, struct list_head *orig) +{ + struct dev_exception_item *ex, *tmp; + + lockdep_assert_held(&devcgroup_mutex); + + list_for_each_entry_safe(ex, tmp, orig, list) { + list_move_tail(&ex->list, dest); + } +} + /* * called under devcgroup_mutex */ @@ -604,11 +615,13 @@ static int devcgroup_update_access(struct dev_cgroup *devcgroup, int count, rc = 0; struct dev_exception_item ex; struct dev_cgroup *parent = css_to_devcgroup(devcgroup->css.parent); + struct dev_cgroup tmp_devcgrp; if (!capable(CAP_SYS_ADMIN)) return -EPERM; memset(&ex, 0, sizeof(ex)); + memset(&tmp_devcgrp, 0, sizeof(tmp_devcgrp)); b = buffer; switch (*b) { @@ -620,15 +633,27 @@ static int devcgroup_update_access(struct dev_cgroup *devcgroup, if (!may_allow_all(parent)) return -EPERM; - dev_exception_clean(devcgroup); - devcgroup->behavior = DEVCG_DEFAULT_ALLOW; - if (!parent) + if (!parent) { + devcgroup->behavior = DEVCG_DEFAULT_ALLOW; + dev_exception_clean(devcgroup); break; + } + INIT_LIST_HEAD(&tmp_devcgrp.exceptions); + rc = dev_exceptions_copy(&tmp_devcgrp.exceptions, + &devcgroup->exceptions); + if (rc) + return rc; + dev_exception_clean(devcgroup); rc = dev_exceptions_copy(&devcgroup->exceptions, &parent->exceptions); - if (rc) + if (rc) { + dev_exceptions_move(&devcgroup->exceptions, + &tmp_devcgrp.exceptions); return rc; + } + devcgroup->behavior = DEVCG_DEFAULT_ALLOW; + dev_exception_clean(&tmp_devcgrp); break; case DEVCG_DENY: if (css_has_online_children(&devcgroup->css)) diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 708de9656bbd..fa5ff13fa8c9 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -335,14 +335,15 @@ static int evm_is_immutable(struct dentry *dentry, struct inode *inode) (char **)&xattr_data, 0, GFP_NOFS); if (rc <= 0) { if (rc == -ENODATA) - return 0; - return rc; + rc = 0; + goto out; } if (xattr_data->type == EVM_XATTR_PORTABLE_DIGSIG) rc = 1; else rc = 0; +out: kfree(xattr_data); return rc; } diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index e01cfd4ad896..f02e609460e2 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -455,14 +455,17 @@ static int evm_xattr_change(struct user_namespace *mnt_userns, rc = vfs_getxattr_alloc(&init_user_ns, dentry, xattr_name, &xattr_data, 0, GFP_NOFS); - if (rc < 0) - return 1; + if (rc < 0) { + rc = 1; + goto out; + } if (rc == xattr_value_len) rc = !!memcmp(xattr_value, xattr_data, rc); else rc = 1; +out: kfree(xattr_data); return rc; } diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index be965a8715e4..03b440921e61 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -326,7 +326,7 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint, enum hash_algo ima_get_hash_algo(const struct evm_ima_xattr_data *xattr_value, int xattr_len); int ima_read_xattr(struct dentry *dentry, - struct evm_ima_xattr_data **xattr_value); + struct evm_ima_xattr_data **xattr_value, int xattr_len); #else static inline int ima_check_blacklist(struct integrity_iint_cache *iint, @@ -372,7 +372,8 @@ ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len) } static inline int ima_read_xattr(struct dentry *dentry, - struct evm_ima_xattr_data **xattr_value) + struct evm_ima_xattr_data **xattr_value, + int xattr_len) { return 0; } diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 3c9af3dc0713..ee6f7e237f2e 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -221,12 +221,12 @@ enum hash_algo ima_get_hash_algo(const struct evm_ima_xattr_data *xattr_value, } int ima_read_xattr(struct dentry *dentry, - struct evm_ima_xattr_data **xattr_value) + struct evm_ima_xattr_data **xattr_value, int xattr_len) { - ssize_t ret; + int ret; ret = vfs_getxattr_alloc(&init_user_ns, dentry, XATTR_NAME_IMA, - (char **)xattr_value, 0, GFP_NOFS); + (char **)xattr_value, xattr_len, GFP_NOFS); if (ret == -EOPNOTSUPP) ret = 0; return ret; diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 040b03ddc1c7..0226899947d6 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -293,7 +293,8 @@ static int process_measurement(struct file *file, const struct cred *cred, /* HASH sets the digital signature and update flags, nothing else */ if ((action & IMA_HASH) && !(test_bit(IMA_DIGSIG, &iint->atomic_flags))) { - xattr_len = ima_read_xattr(file_dentry(file), &xattr_value); + xattr_len = ima_read_xattr(file_dentry(file), + &xattr_value, xattr_len); if ((xattr_value && xattr_len > 2) && (xattr_value->type == EVM_IMA_XATTR_DIGSIG)) set_bit(IMA_DIGSIG, &iint->atomic_flags); @@ -316,7 +317,8 @@ static int process_measurement(struct file *file, const struct cred *cred, if ((action & IMA_APPRAISE_SUBMASK) || strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0) { /* read 'security.ima' */ - xattr_len = ima_read_xattr(file_dentry(file), &xattr_value); + xattr_len = ima_read_xattr(file_dentry(file), + &xattr_value, xattr_len); /* * Read the appended modsig if allowed by the policy, and allow diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c index 7bf9b1507220..4564faae7d67 100644 --- a/security/integrity/ima/ima_template_lib.c +++ b/security/integrity/ima/ima_template_lib.c @@ -601,16 +601,15 @@ int ima_eventevmsig_init(struct ima_event_data *event_data, rc = vfs_getxattr_alloc(&init_user_ns, file_dentry(event_data->file), XATTR_NAME_EVM, (char **)&xattr_data, 0, GFP_NOFS); - if (rc <= 0) - return 0; - - if (xattr_data->type != EVM_XATTR_PORTABLE_DIGSIG) { - kfree(xattr_data); - return 0; + if (rc <= 0 || xattr_data->type != EVM_XATTR_PORTABLE_DIGSIG) { + rc = 0; + goto out; } rc = ima_write_template_field_data((char *)xattr_data, rc, DATA_FMT_HEX, field_data); + +out: kfree(xattr_data); return rc; } diff --git a/security/lsm_audit.c b/security/lsm_audit.c index 75cc3f8d2a42..a7355b4b9bb8 100644 --- a/security/lsm_audit.c +++ b/security/lsm_audit.c @@ -190,6 +190,7 @@ static inline void print_ipv4_addr(struct audit_buffer *ab, __be32 addr, /** * dump_common_audit_data - helper to dump common audit data + * @ab : the audit buffer * @a : common audit data * */ diff --git a/security/security.c b/security/security.c index b967e035b456..d1571900a8c7 100644 --- a/security/security.c +++ b/security/security.c @@ -161,8 +161,8 @@ static void __init append_ordered_lsm(struct lsm_info *lsm, const char *from) lsm->enabled = &lsm_enabled_true; ordered_lsms[last_lsm++] = lsm; - init_debug("%s ordering: %s (%sabled)\n", from, lsm->name, - is_enabled(lsm) ? "en" : "dis"); + init_debug("%s ordered: %s (%s)\n", from, lsm->name, + is_enabled(lsm) ? "enabled" : "disabled"); } /* Is an LSM allowed to be initialized? */ @@ -225,7 +225,7 @@ static void __init prepare_lsm(struct lsm_info *lsm) if (enabled) { if ((lsm->flags & LSM_FLAG_EXCLUSIVE) && !exclusive) { exclusive = lsm; - init_debug("exclusive chosen: %s\n", lsm->name); + init_debug("exclusive chosen: %s\n", lsm->name); } lsm_set_blob_sizes(lsm->blobs); @@ -253,7 +253,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin) /* LSM_ORDER_FIRST is always first. */ for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) { if (lsm->order == LSM_ORDER_FIRST) - append_ordered_lsm(lsm, "first"); + append_ordered_lsm(lsm, " first"); } /* Process "security=", if given. */ @@ -271,7 +271,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin) if ((major->flags & LSM_FLAG_LEGACY_MAJOR) && strcmp(major->name, chosen_major_lsm) != 0) { set_enabled(major, false); - init_debug("security=%s disabled: %s\n", + init_debug("security=%s disabled: %s (only one legacy major LSM)\n", chosen_major_lsm, major->name); } } @@ -292,7 +292,8 @@ static void __init ordered_lsm_parse(const char *order, const char *origin) } if (!found) - init_debug("%s ignored: %s\n", origin, name); + init_debug("%s ignored: %s (not built into kernel)\n", + origin, name); } /* Process "security=", if given. */ @@ -310,7 +311,8 @@ static void __init ordered_lsm_parse(const char *order, const char *origin) if (exists_ordered_lsm(lsm)) continue; set_enabled(lsm, false); - init_debug("%s disabled: %s\n", origin, lsm->name); + init_debug("%s skipped: %s (not in requested order)\n", + origin, lsm->name); } kfree(sep); @@ -321,6 +323,24 @@ static void __init lsm_early_task(struct task_struct *task); static int lsm_append(const char *new, char **result); +static void __init report_lsm_order(void) +{ + struct lsm_info **lsm, *early; + int first = 0; + + pr_info("initializing lsm="); + + /* Report each enabled LSM name, comma separated. */ + for (early = __start_early_lsm_info; early < __end_early_lsm_info; early++) + if (is_enabled(early)) + pr_cont("%s%s", first++ == 0 ? "" : ",", early->name); + for (lsm = ordered_lsms; *lsm; lsm++) + if (is_enabled(*lsm)) + pr_cont("%s%s", first++ == 0 ? "" : ",", (*lsm)->name); + + pr_cont("\n"); +} + static void __init ordered_lsm_init(void) { struct lsm_info **lsm; @@ -330,7 +350,8 @@ static void __init ordered_lsm_init(void) if (chosen_lsm_order) { if (chosen_major_lsm) { - pr_info("security= is ignored because it is superseded by lsm=\n"); + pr_warn("security=%s is ignored because it is superseded by lsm=%s\n", + chosen_major_lsm, chosen_lsm_order); chosen_major_lsm = NULL; } ordered_lsm_parse(chosen_lsm_order, "cmdline"); @@ -340,6 +361,8 @@ static void __init ordered_lsm_init(void) for (lsm = ordered_lsms; *lsm; lsm++) prepare_lsm(*lsm); + report_lsm_order(); + init_debug("cred blob size = %d\n", blob_sizes.lbs_cred); init_debug("file blob size = %d\n", blob_sizes.lbs_file); init_debug("inode blob size = %d\n", blob_sizes.lbs_inode); @@ -396,13 +419,17 @@ int __init security_init(void) { struct lsm_info *lsm; - pr_info("Security Framework initializing\n"); + init_debug("legacy security=%s\n", chosen_major_lsm ?: " *unspecified*"); + init_debug(" CONFIG_LSM=%s\n", builtin_lsm_order); + init_debug("boot arg lsm=%s\n", chosen_lsm_order ?: " *unspecified*"); /* * Append the names of the early LSM modules now that kmalloc() is * available */ for (lsm = __start_early_lsm_info; lsm < __end_early_lsm_info; lsm++) { + init_debug(" early started: %s (%s)\n", lsm->name, + is_enabled(lsm) ? "enabled" : "disabled"); if (lsm->enabled) lsm_append(lsm->name, &lsm_names); } @@ -2315,11 +2342,11 @@ int security_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) } EXPORT_SYMBOL(security_sock_rcv_skb); -int security_socket_getpeersec_stream(struct socket *sock, char __user *optval, - int __user *optlen, unsigned len) +int security_socket_getpeersec_stream(struct socket *sock, sockptr_t optval, + sockptr_t optlen, unsigned int len) { return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock, - optval, optlen, len); + optval, optlen, len); } int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 7c5c8d17695c..3c5be76a9199 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -5138,11 +5138,12 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) return err; } -static int selinux_socket_getpeersec_stream(struct socket *sock, char __user *optval, - int __user *optlen, unsigned len) +static int selinux_socket_getpeersec_stream(struct socket *sock, + sockptr_t optval, sockptr_t optlen, + unsigned int len) { int err = 0; - char *scontext; + char *scontext = NULL; u32 scontext_len; struct sk_security_struct *sksec = sock->sk->sk_security; u32 peer_sid = SECSID_NULL; @@ -5158,17 +5159,15 @@ static int selinux_socket_getpeersec_stream(struct socket *sock, char __user *op &scontext_len); if (err) return err; - if (scontext_len > len) { err = -ERANGE; goto out_len; } - if (copy_to_user(optval, scontext, scontext_len)) + if (copy_to_sockptr(optval, scontext, scontext_len)) err = -EFAULT; - out_len: - if (put_user(scontext_len, optlen)) + if (copy_to_sockptr(optlen, &scontext_len, sizeof(scontext_len))) err = -EFAULT; kfree(scontext); return err; diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index cadef2f6a75e..9a82a15685d1 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -4074,12 +4074,12 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) * returns zero on success, an error code otherwise */ static int smack_socket_getpeersec_stream(struct socket *sock, - char __user *optval, - int __user *optlen, unsigned len) + sockptr_t optval, sockptr_t optlen, + unsigned int len) { struct socket_smack *ssp; char *rcp = ""; - int slen = 1; + u32 slen = 1; int rc = 0; ssp = sock->sk->sk_security; @@ -4087,15 +4087,16 @@ static int smack_socket_getpeersec_stream(struct socket *sock, rcp = ssp->smk_packet->smk_known; slen = strlen(rcp) + 1; } - - if (slen > len) + if (slen > len) { rc = -ERANGE; - else if (copy_to_user(optval, rcp, slen) != 0) - rc = -EFAULT; + goto out_len; + } - if (put_user(slen, optlen) != 0) + if (copy_to_sockptr(optval, rcp, slen)) + rc = -EFAULT; +out_len: + if (copy_to_sockptr(optlen, &slen, sizeof(slen))) rc = -EFAULT; - return rc; } |