checkreqprot data member in selinux_state struct is accessed directly by SELinux functions to get and set. This could cause unexpected read or write access to this data member due to compiler optimizations and/or compiler's reordering of access to this field. Add helper functions to get and set checkreqprot data member in selinux_state struct. These helper functions use READ_ONCE and WRITE_ONCE macros to ensure atomic read or write of memory for this data member. Signed-off-by: Lakshmi Ramasubramanian <nramas@xxxxxxxxxxxxxxxxxxx> Suggested-by: Stephen Smalley <stephen.smalley.work@xxxxxxxxx> --- security/selinux/hooks.c | 6 +++--- security/selinux/include/security.h | 10 ++++++++++ security/selinux/selinuxfs.c | 5 +++-- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 6210e98219a5..25a36a3e6bed 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3718,7 +3718,7 @@ static int selinux_mmap_file(struct file *file, unsigned long reqprot, return rc; } - if (selinux_state.checkreqprot) + if (checkreqprot_enabled(&selinux_state)) prot = reqprot; return file_map_prot_check(file, prot, @@ -3732,7 +3732,7 @@ static int selinux_file_mprotect(struct vm_area_struct *vma, const struct cred *cred = current_cred(); u32 sid = cred_sid(cred); - if (selinux_state.checkreqprot) + if (checkreqprot_enabled(&selinux_state)) prot = reqprot; if (default_noexec && @@ -7234,7 +7234,7 @@ static __init int selinux_init(void) memset(&selinux_state, 0, sizeof(selinux_state)); enforcing_set(&selinux_state, selinux_enforcing_boot); - selinux_state.checkreqprot = selinux_checkreqprot_boot; + checkreqprot_set(&selinux_state, selinux_checkreqprot_boot); selinux_avc_init(&selinux_state.avc); mutex_init(&selinux_state.status_lock); mutex_init(&selinux_state.policy_mutex); diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index cbdd3c7aff8b..cc29177c8858 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -143,6 +143,16 @@ static inline void enforcing_set(struct selinux_state *state, bool value) } #endif +static inline bool checkreqprot_enabled(const struct selinux_state *state) +{ + return READ_ONCE(state->checkreqprot); +} + +static inline void checkreqprot_set(struct selinux_state *state, bool value) +{ + WRITE_ONCE(state->checkreqprot, value); +} + #ifdef CONFIG_SECURITY_SELINUX_DISABLE static inline bool selinux_disabled(struct selinux_state *state) { diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 45e9efa9bf5b..540bfa078877 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -717,7 +717,8 @@ static ssize_t sel_read_checkreqprot(struct file *filp, char __user *buf, char tmpbuf[TMPBUFLEN]; ssize_t length; - length = scnprintf(tmpbuf, TMPBUFLEN, "%u", fsi->state->checkreqprot); + length = scnprintf(tmpbuf, TMPBUFLEN, "%u", + checkreqprot_enabled(fsi->state)); return simple_read_from_buffer(buf, count, ppos, tmpbuf, length); } @@ -759,7 +760,7 @@ static ssize_t sel_write_checkreqprot(struct file *file, const char __user *buf, comm, current->pid); } - fsi->state->checkreqprot = new_value ? 1 : 0; + checkreqprot_set(fsi->state, (new_value ? 1 : 0)); length = count; out: kfree(page); -- 2.28.0