On Wed, Sep 9, 2020 at 6:28 PM Lakshmi Ramasubramanian
<nramas@xxxxxxxxxxxxxxxxxxx> 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@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/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);
> +}
This is a nitpick, and I recognize that Stephen already suggested the
use of "*_set()" and "*_enabled()" for names, but if we are going to
name the setter "*_set()" let's also name the getter "*_get()".
Other than that, it looks fine to me.
--
paul moore
www.paul-moore.com
