| Message ID | 20190125100651.21753-2-omosnace@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Report raw context in AVCs + refactoring | expand |
On 1/25/19 5:06 AM, Ondrej Mosnacek 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> Reviewed-by: Stephen Smalley <sds@tycho.nsa.gov> > --- > security/selinux/avc.c | 140 +++++++++++++++++------------------------ > 1 file changed, 58 insertions(+), 82 deletions(-) > > 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 */ >
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 >
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 */
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(-)