On Tue, Aug 25, 2020 at 3:00 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote:
>
> There seems to be no reason to use GFP_ATOMIC in these cases.
>
> Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
> ---
> security/selinux/hooks.c | 6 +++---
> security/selinux/ss/policydb.c | 10 +++++-----
> security/selinux/ss/services.c | 4 ++--
> 3 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 89d3753b7bd5d..4de962daffbde 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6854,7 +6854,7 @@ static int selinux_lockdown(enum lockdown_reason what)
>
> if (WARN(invalid_reason, "Invalid lockdown reason")) {
> audit_log(audit_context(),
> - GFP_ATOMIC, AUDIT_SELINUX_ERR,
> + GFP_KERNEL, AUDIT_SELINUX_ERR,
> "lockdown_reason=invalid");
> return -EINVAL;
> }
Have you audited all callers of security_locked_down() to ensure that
they are never holding any locks around the call? That's the only one
I saw that might be a problem now or in the future.
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index a48fc1b337ba9..fa61a54bc1440 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -109,7 +109,7 @@ static int selinux_set_mapping(struct policydb *pol,
> @@ -2982,7 +2982,7 @@ int security_set_bools(struct selinux_state *state, u32 len, int *values)
> int old_state = newpolicy->policydb.bool_val_to_struct[i]->state;
>
> if (new_state != old_state) {
> - audit_log(audit_context(), GFP_ATOMIC,
> + audit_log(audit_context(), GFP_KERNEL,
> AUDIT_MAC_CONFIG_CHANGE,
> "bool=%s val=%d old_val=%d auid=%u ses=%u",
> sym_name(&newpolicy->policydb, SYM_BOOLS, i),
Note that this one shouldn't be back-ported prior to my refactoring
patches because previously this was called while holding the policy
rwlock.
