Re: [PATCH v3 1/4] selinux: inline some AVC functions used only once

[Paul Moore <paul@paul-moore.com>]

On Fri, Jan 25, 2019 at 5:07 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> avc_dump_av() and avc_dump_query() are each used only in one place. Get
> rid of them and open code their contents in the call sites.
>
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
>  security/selinux/avc.c | 140 +++++++++++++++++------------------------
>  1 file changed, 58 insertions(+), 82 deletions(-)

Merged, thanks.

> diff --git a/security/selinux/avc.c b/security/selinux/avc.c
> index 9b63d8ee1687..502162eeb3a0 100644
> --- a/security/selinux/avc.c
> +++ b/security/selinux/avc.c
> @@ -129,75 +129,6 @@ static inline int avc_hash(u32 ssid, u32 tsid, u16 tclass)
>         return (ssid ^ (tsid<<2) ^ (tclass<<4)) & (AVC_CACHE_SLOTS - 1);
>  }
>
> -/**
> - * avc_dump_av - Display an access vector in human-readable form.
> - * @tclass: target security class
> - * @av: access vector
> - */
> -static void avc_dump_av(struct audit_buffer *ab, u16 tclass, u32 av)
> -{
> -       const char **perms;
> -       int i, perm;
> -
> -       if (av == 0) {
> -               audit_log_format(ab, " null");
> -               return;
> -       }
> -
> -       BUG_ON(!tclass || tclass >= ARRAY_SIZE(secclass_map));
> -       perms = secclass_map[tclass-1].perms;
> -
> -       audit_log_format(ab, " {");
> -       i = 0;
> -       perm = 1;
> -       while (i < (sizeof(av) * 8)) {
> -               if ((perm & av) && perms[i]) {
> -                       audit_log_format(ab, " %s", perms[i]);
> -                       av &= ~perm;
> -               }
> -               i++;
> -               perm <<= 1;
> -       }
> -
> -       if (av)
> -               audit_log_format(ab, " 0x%x", av);
> -
> -       audit_log_format(ab, " }");
> -}
> -
> -/**
> - * avc_dump_query - Display a SID pair and a class in human-readable form.
> - * @ssid: source security identifier
> - * @tsid: target security identifier
> - * @tclass: target security class
> - */
> -static void avc_dump_query(struct audit_buffer *ab, struct selinux_state *state,
> -                          u32 ssid, u32 tsid, u16 tclass)
> -{
> -       int rc;
> -       char *scontext;
> -       u32 scontext_len;
> -
> -       rc = security_sid_to_context(state, ssid, &scontext, &scontext_len);
> -       if (rc)
> -               audit_log_format(ab, "ssid=%d", ssid);
> -       else {
> -               audit_log_format(ab, "scontext=%s", scontext);
> -               kfree(scontext);
> -       }
> -
> -       rc = security_sid_to_context(state, tsid, &scontext, &scontext_len);
> -       if (rc)
> -               audit_log_format(ab, " tsid=%d", tsid);
> -       else {
> -               audit_log_format(ab, " tcontext=%s", scontext);
> -               kfree(scontext);
> -       }
> -
> -       BUG_ON(!tclass || tclass >= ARRAY_SIZE(secclass_map));
> -       audit_log_format(ab, " tclass=%s", secclass_map[tclass-1].name);
> -}
> -
>  /**
>   * avc_init - Initialize the AVC.
>   *
> @@ -735,11 +666,37 @@ out:
>  static void avc_audit_pre_callback(struct audit_buffer *ab, void *a)
>  {
>         struct common_audit_data *ad = a;
> -       audit_log_format(ab, "avc:  %s ",
> -                        ad->selinux_audit_data->denied ? "denied" : "granted");
> -       avc_dump_av(ab, ad->selinux_audit_data->tclass,
> -                       ad->selinux_audit_data->audited);
> -       audit_log_format(ab, " for ");
> +       struct selinux_audit_data *sad = ad->selinux_audit_data;
> +       u32 av = sad->audited;
> +       const char **perms;
> +       int i, perm;
> +
> +       audit_log_format(ab, "avc:  %s ", sad->denied ? "denied" : "granted");
> +
> +       if (av == 0) {
> +               audit_log_string(ab, " null");
> +               return;
> +       }
> +
> +       BUG_ON(!sad->tclass || sad->tclass >= ARRAY_SIZE(secclass_map));
> +       perms = secclass_map[sad->tclass-1].perms;
> +
> +       audit_log_string(ab, " {");
> +       i = 0;
> +       perm = 1;
> +       while (i < (sizeof(av) * 8)) {
> +               if ((perm & av) && perms[i]) {
> +                       audit_log_format(ab, " %s", perms[i]);
> +                       av &= ~perm;
> +               }
> +               i++;
> +               perm <<= 1;
> +       }
> +
> +       if (av)
> +               audit_log_format(ab, " 0x%x", av);
> +
> +       audit_log_string(ab, " } for ");
>  }
>
>  /**
> @@ -751,15 +708,34 @@ static void avc_audit_pre_callback(struct audit_buffer *ab, void *a)
>  static void avc_audit_post_callback(struct audit_buffer *ab, void *a)
>  {
>         struct common_audit_data *ad = a;
> -       audit_log_format(ab, " ");
> -       avc_dump_query(ab, ad->selinux_audit_data->state,
> -                      ad->selinux_audit_data->ssid,
> -                      ad->selinux_audit_data->tsid,
> -                      ad->selinux_audit_data->tclass);
> -       if (ad->selinux_audit_data->denied) {
> -               audit_log_format(ab, " permissive=%u",
> -                                ad->selinux_audit_data->result ? 0 : 1);
> +       struct selinux_audit_data *sad = ad->selinux_audit_data;
> +       char *scontext;
> +       u32 scontext_len;
> +       int rc;
> +
> +       rc = security_sid_to_context(sad->state, sad->ssid, &scontext,
> +                                    &scontext_len);
> +       if (rc)
> +               audit_log_format(ab, " ssid=%d", sad->ssid);
> +       else {
> +               audit_log_format(ab, " scontext=%s", scontext);
> +               kfree(scontext);
>         }
> +
> +       rc = security_sid_to_context(sad->state, sad->tsid, &scontext,
> +                                    &scontext_len);
> +       if (rc)
> +               audit_log_format(ab, " tsid=%d", sad->tsid);
> +       else {
> +               audit_log_format(ab, " tcontext=%s", scontext);
> +               kfree(scontext);
> +       }
> +
> +       BUG_ON(!sad->tclass || sad->tclass >= ARRAY_SIZE(secclass_map));
> +       audit_log_format(ab, " tclass=%s", secclass_map[sad->tclass-1].name);
> +
> +       if (sad->denied)
> +               audit_log_format(ab, " permissive=%u", sad->result ? 0 : 1);
>  }
>
>  /* This is the slow part of avc audit with big stack footprint */
> --
> 2.20.1
>


-- 
paul moore
www.paul-moore.com