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 explicit read or write of memory for
this data member.
This patch is based on commit 66ccd2560aff
("selinux: simplify away security_policydb_len()") in "next" branch
in https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
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..520c7b78bcd8 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 (selinux_checkreqprot(&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 (selinux_checkreqprot(&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;
+ selinux_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..b19d919f01e7 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -209,6 +209,16 @@ static inline bool selinux_policycap_genfs_seclabel_symlinks(void)
return state->policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS];
}
+static inline bool selinux_checkreqprot(const struct selinux_state *state)
+{
+ return READ_ONCE(state->checkreqprot);
+}
+static inline void selinux_checkreqprot_set(struct selinux_state *state,
+ bool value)
+{
+ WRITE_ONCE(state->checkreqprot, value);
+}
+
int security_mls_enabled(struct selinux_state *state);
int security_load_policy(struct selinux_state *state,
void *data, size_t len,
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 45e9efa9bf5b..ba636d3ea89d 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",
+ selinux_checkreqprot(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;
+ selinux_checkreqprot_set(fsi->state, (new_value ? 1 : 0));
length = count;
out:
kfree(page);
--
2.28.0
