| Message ID | 20200914173157.4655-1-nramas@linux.microsoft.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v4] selinux: Add helper functions to get and set checkreqprot | expand |
On Mon, Sep 14, 2020 at 1:32 PM Lakshmi Ramasubramanian <nramas@linux.microsoft.com> wrote: > > 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@linux.microsoft.com> > Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com> > Suggested-by: Paul Moore <paul@paul-moore.com> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
On Mon, Sep 14, 2020 at 1:32 PM Lakshmi Ramasubramanian <nramas@linux.microsoft.com> wrote: > > 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@linux.microsoft.com> > Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com> > Suggested-by: Paul Moore <paul@paul-moore.com> > --- > security/selinux/hooks.c | 6 +++--- > security/selinux/include/security.h | 10 ++++++++++ > security/selinux/selinuxfs.c | 5 +++-- > 3 files changed, 16 insertions(+), 5 deletions(-) Merged into selinux/next, thanks Lakshmi!
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 6210e98219a5..d6b182c11700 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_get(&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_get(&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 0ce2ef684ed0..3cc8bab31ea8 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_get(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..4bde570d56a2 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_get(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);
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@linux.microsoft.com> Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com> Suggested-by: Paul Moore <paul@paul-moore.com> --- security/selinux/hooks.c | 6 +++--- security/selinux/include/security.h | 10 ++++++++++ security/selinux/selinuxfs.c | 5 +++-- 3 files changed, 16 insertions(+), 5 deletions(-)