summaryrefslogtreecommitdiff
path: root/security/selinux/include
diff options
context:
space:
mode:
authorOndrej Mosnacek <omosnace@redhat.com>2021-03-19 00:53:02 +0300
committerPaul Moore <paul@paul-moore.com>2021-03-19 06:23:46 +0300
commit6406887a12ee5dcdaffff1a8508d91113d545559 (patch)
tree330e72853653a1f8071af6e1cc304ae3ceae304e /security/selinux/include
parent519dad3bcd809dc1523bf80ab0310ddb3bf00ade (diff)
downloadlinux-6406887a12ee5dcdaffff1a8508d91113d545559.tar.xz
selinux: fix variable scope issue in live sidtab conversion
Commit 02a52c5c8c3b ("selinux: move policy commit after updating selinuxfs") moved the selinux_policy_commit() call out of security_load_policy() into sel_write_load(), which caused a subtle yet rather serious bug. The problem is that security_load_policy() passes a reference to the convert_params local variable to sidtab_convert(), which stores it in the sidtab, where it may be accessed until the policy is swapped over and RCU synchronized. Before 02a52c5c8c3b, selinux_policy_commit() was called directly from security_load_policy(), so the convert_params pointer remained valid all the way until the old sidtab was destroyed, but now that's no longer the case and calls to sidtab_context_to_sid() on the old sidtab after security_load_policy() returns may cause invalid memory accesses. This can be easily triggered using the stress test from commit ee1a84fdfeed ("selinux: overhaul sidtab to fix bug and improve performance"): ``` function rand_cat() { echo $(( $RANDOM % 1024 )) } function do_work() { while true; do echo -n "system_u:system_r:kernel_t:s0:c$(rand_cat),c$(rand_cat)" \ >/sys/fs/selinux/context 2>/dev/null || true done } do_work >/dev/null & do_work >/dev/null & do_work >/dev/null & while load_policy; do echo -n .; sleep 0.1; done kill %1 kill %2 kill %3 ``` Fix this by allocating the temporary sidtab convert structures dynamically and passing them among the selinux_policy_{load,cancel,commit} functions. Fixes: 02a52c5c8c3b ("selinux: move policy commit after updating selinuxfs") Cc: stable@vger.kernel.org Tested-by: Tyler Hicks <tyhicks@linux.microsoft.com> Reviewed-by: Tyler Hicks <tyhicks@linux.microsoft.com> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> [PM: merge fuzz in security.h and services.c] Signed-off-by: Paul Moore <paul@paul-moore.com>
Diffstat (limited to 'security/selinux/include')
-rw-r--r--security/selinux/include/security.h15
1 files changed, 11 insertions, 4 deletions
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 765a258a899e..25db66e0ac51 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -219,14 +219,21 @@ static inline bool selinux_policycap_genfs_seclabel_symlinks(void)
return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS]);
}
+struct selinux_policy_convert_data;
+
+struct selinux_load_state {
+ struct selinux_policy *policy;
+ struct selinux_policy_convert_data *convert_data;
+};
+
int security_mls_enabled(struct selinux_state *state);
int security_load_policy(struct selinux_state *state,
- void *data, size_t len,
- struct selinux_policy **newpolicyp);
+ void *data, size_t len,
+ struct selinux_load_state *load_state);
void selinux_policy_commit(struct selinux_state *state,
- struct selinux_policy *newpolicy);
+ struct selinux_load_state *load_state);
void selinux_policy_cancel(struct selinux_state *state,
- struct selinux_policy *policy);
+ struct selinux_load_state *load_state);
int security_read_policy(struct selinux_state *state,
void **data, size_t *len);