| Message ID | 20200813144914.737306-2-tweek@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/2] selinux: add tracepoint on denials | expand |
On 8/13/2020 7:48 AM, Thiébaud Weksteen wrote: > From: Peter Enderborg <peter.enderborg@sony.com> > > This patch adds further attributes to the event. These attributes are > helpful to understand the context of the message and can be used > to filter the events. > > There are three common items. Source context, target context and tclass. > There are also items from the outcome of operation performed. > > An event is similar to: > <...>-1309 [002] .... 6346.691689: selinux_audited: > requested=0x4000000 denied=0x4000000 audited=0x4000000 > result=-13 ssid=315 tsid=61 It may not be my place to ask, but *please please please* don't externalize secids. I understand that it's easier to type "42" than "system_r:cupsd_t:s0-s0:c0.c1023", and that it's easier for your tools to parse and store the number. Once you start training people that system_r:cupsd_t:s0-s0:c0.c1023 is secid 42 you'll never be able to change it. The secid will start showing up in scripts. Bad Things Will Happen. > scontext=system_u:system_r:cupsd_t:s0-s0:c0.c1023 > tcontext=system_u:object_r:bin_t:s0 tclass=file > > With systems where many denials are occurring, it is useful to apply a > filter. The filtering is a set of logic that is inserted with > the filter file. Example: > echo "tclass==\"file\" && ssid!=42" > events/avc/selinux_audited/filter > > This adds that we only get tclass=file but not for ssid 42. > > The trace can also have extra properties. Adding the user stack > can be done with > echo 1 > options/userstacktrace > > Now the output will be > runcon-1365 [003] .... 6960.955530: selinux_audited: > requested=0x4000000 denied=0x4000000 audited=0x4000000 > result=-13 ssid=315 tsid=61 > scontext=system_u:system_r:cupsd_t:s0-s0:c0.c1023 > tcontext=system_u:object_r:bin_t:s0 tclass=file > runcon-1365 [003] .... 6960.955560: <user stack trace> > => <00007f325b4ce45b> > => <00005607093efa57> > > Note that the ssid is the internal numeric representation of scontext > and tsid is numeric for tcontext. They are useful for filtering. > > Signed-off-by: Peter Enderborg <peter.enderborg@sony.com> > Reviewed-by: Thiébaud Weksteen <tweek@google.com> > --- > v2 changes: > - update changelog to include usage examples > > include/trace/events/avc.h | 41 ++++++++++++++++++++++++++++---------- > security/selinux/avc.c | 22 +++++++++++--------- > 2 files changed, 44 insertions(+), 19 deletions(-) > > diff --git a/include/trace/events/avc.h b/include/trace/events/avc.h > index 07c058a9bbcd..ac5ef2e1c2c5 100644 > --- a/include/trace/events/avc.h > +++ b/include/trace/events/avc.h > @@ -1,6 +1,7 @@ > /* SPDX-License-Identifier: GPL-2.0 */ > /* > - * Author: Thiébaud Weksteen <tweek@google.com> > + * Authors: Thiébaud Weksteen <tweek@google.com> > + * Peter Enderborg <Peter.Enderborg@sony.com> > */ > #undef TRACE_SYSTEM > #define TRACE_SYSTEM avc > @@ -12,23 +13,43 @@ > > TRACE_EVENT(selinux_audited, > > - TP_PROTO(struct selinux_audit_data *sad), > + TP_PROTO(struct selinux_audit_data *sad, > + char *scontext, > + char *tcontext, > + const char *tclass > + ), > > - TP_ARGS(sad), > + TP_ARGS(sad, scontext, tcontext, tclass), > > TP_STRUCT__entry( > - __field(unsigned int, tclass) > - __field(unsigned int, audited) > + __field(u32, requested) > + __field(u32, denied) > + __field(u32, audited) > + __field(int, result) > + __string(scontext, scontext) > + __string(tcontext, tcontext) > + __string(tclass, tclass) > + __field(u32, ssid) > + __field(u32, tsid) > ), > > TP_fast_assign( > - __entry->tclass = sad->tclass; > - __entry->audited = sad->audited; > + __entry->requested = sad->requested; > + __entry->denied = sad->denied; > + __entry->audited = sad->audited; > + __entry->result = sad->result; > + __entry->ssid = sad->ssid; > + __entry->tsid = sad->tsid; > + __assign_str(tcontext, tcontext); > + __assign_str(scontext, scontext); > + __assign_str(tclass, tclass); > ), > > - TP_printk("tclass=%u audited=%x", > - __entry->tclass, > - __entry->audited) > + TP_printk("requested=0x%x denied=0x%x audited=0x%x result=%d ssid=%u tsid=%u scontext=%s tcontext=%s tclass=%s", > + __entry->requested, __entry->denied, __entry->audited, __entry->result, > + __entry->ssid, __entry->tsid, __get_str(scontext), __get_str(tcontext), > + __get_str(tclass) > + ) > ); > > #endif > diff --git a/security/selinux/avc.c b/security/selinux/avc.c > index b0a0af778b70..7de5cc5169af 100644 > --- a/security/selinux/avc.c > +++ b/security/selinux/avc.c > @@ -705,35 +705,39 @@ static void avc_audit_post_callback(struct audit_buffer *ab, void *a) > { > struct common_audit_data *ad = a; > struct selinux_audit_data *sad = ad->selinux_audit_data; > - char *scontext; > + char *scontext = NULL; > + char *tcontext = NULL; > + const char *tclass = NULL; > u32 scontext_len; > + u32 tcontext_len; > int rc; > > - trace_selinux_audited(sad); > - > 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); > + rc = security_sid_to_context(sad->state, sad->tsid, &tcontext, > + &tcontext_len); > if (rc) > audit_log_format(ab, " tsid=%d", sad->tsid); > else { > - audit_log_format(ab, " tcontext=%s", scontext); > - kfree(scontext); > + audit_log_format(ab, " tcontext=%s", tcontext); > } > > - audit_log_format(ab, " tclass=%s", secclass_map[sad->tclass-1].name); > + tclass = secclass_map[sad->tclass-1].name; > + audit_log_format(ab, " tclass=%s", tclass); > > if (sad->denied) > audit_log_format(ab, " permissive=%u", sad->result ? 0 : 1); > > + trace_selinux_audited(sad, scontext, tcontext, tclass); > + kfree(tcontext); > + kfree(scontext); > + > /* in case of invalid context report also the actual context string */ > rc = security_sid_to_context_inval(sad->state, sad->ssid, &scontext, > &scontext_len);
On 8/13/20 5:05 PM, Casey Schaufler wrote: > On 8/13/2020 7:48 AM, Thiébaud Weksteen wrote: >> From: Peter Enderborg <peter.enderborg@sony.com> >> >> This patch adds further attributes to the event. These attributes are >> helpful to understand the context of the message and can be used >> to filter the events. >> >> There are three common items. Source context, target context and tclass. >> There are also items from the outcome of operation performed. >> >> An event is similar to: >> <...>-1309 [002] .... 6346.691689: selinux_audited: >> requested=0x4000000 denied=0x4000000 audited=0x4000000 >> result=-13 ssid=315 tsid=61 > It may not be my place to ask, but *please please please* don't > externalize secids. I understand that it's easier to type "42" > than "system_r:cupsd_t:s0-s0:c0.c1023", and that it's easier for > your tools to parse and store the number. Once you start training > people that system_r:cupsd_t:s0-s0:c0.c1023 is secid 42 you'll > never be able to change it. The secid will start showing up in > scripts. Bad Things Will Happen. Ok, it seems to mostly against having this performance options. Yes, it is a kernel internal data. So is most of the kernel tracing. I see it is a primary tool for kernel debugging but than can also be used for user-space debugging tools. Hiding data for debuggers does not make any sense too me. >> scontext=system_u:system_r:cupsd_t:s0-s0:c0.c1023 >> tcontext=system_u:object_r:bin_t:s0 tclass=file >> >> With systems where many denials are occurring, it is useful to apply a >> filter. The filtering is a set of logic that is inserted with >> the filter file. Example: >> echo "tclass==\"file\" && ssid!=42" > events/avc/selinux_audited/filter >> >> This adds that we only get tclass=file but not for ssid 42. >> >> The trace can also have extra properties. Adding the user stack >> can be done with >> echo 1 > options/userstacktrace >> >> Now the output will be >> runcon-1365 [003] .... 6960.955530: selinux_audited: >> requested=0x4000000 denied=0x4000000 audited=0x4000000 >> result=-13 ssid=315 tsid=61 >> scontext=system_u:system_r:cupsd_t:s0-s0:c0.c1023 >> tcontext=system_u:object_r:bin_t:s0 tclass=file >> runcon-1365 [003] .... 6960.955560: <user stack trace> >> => <00007f325b4ce45b> >> => <00005607093efa57> >> >> Note that the ssid is the internal numeric representation of scontext >> and tsid is numeric for tcontext. They are useful for filtering. >> >> Signed-off-by: Peter Enderborg <peter.enderborg@sony.com> >> Reviewed-by: Thiébaud Weksteen <tweek@google.com> >> --- >> v2 changes: >> - update changelog to include usage examples >> >> include/trace/events/avc.h | 41 ++++++++++++++++++++++++++++---------- >> security/selinux/avc.c | 22 +++++++++++--------- >> 2 files changed, 44 insertions(+), 19 deletions(-) >> >> diff --git a/include/trace/events/avc.h b/include/trace/events/avc.h >> index 07c058a9bbcd..ac5ef2e1c2c5 100644 >> --- a/include/trace/events/avc.h >> +++ b/include/trace/events/avc.h >> @@ -1,6 +1,7 @@ >> /* SPDX-License-Identifier: GPL-2.0 */ >> /* >> - * Author: Thiébaud Weksteen <tweek@google.com> >> + * Authors: Thiébaud Weksteen <tweek@google.com> >> + * Peter Enderborg <Peter.Enderborg@sony.com> >> */ >> #undef TRACE_SYSTEM >> #define TRACE_SYSTEM avc >> @@ -12,23 +13,43 @@ >> >> TRACE_EVENT(selinux_audited, >> >> - TP_PROTO(struct selinux_audit_data *sad), >> + TP_PROTO(struct selinux_audit_data *sad, >> + char *scontext, >> + char *tcontext, >> + const char *tclass >> + ), >> >> - TP_ARGS(sad), >> + TP_ARGS(sad, scontext, tcontext, tclass), >> >> TP_STRUCT__entry( >> - __field(unsigned int, tclass) >> - __field(unsigned int, audited) >> + __field(u32, requested) >> + __field(u32, denied) >> + __field(u32, audited) >> + __field(int, result) >> + __string(scontext, scontext) >> + __string(tcontext, tcontext) >> + __string(tclass, tclass) >> + __field(u32, ssid) >> + __field(u32, tsid) >> ), >> >> TP_fast_assign( >> - __entry->tclass = sad->tclass; >> - __entry->audited = sad->audited; >> + __entry->requested = sad->requested; >> + __entry->denied = sad->denied; >> + __entry->audited = sad->audited; >> + __entry->result = sad->result; >> + __entry->ssid = sad->ssid; >> + __entry->tsid = sad->tsid; >> + __assign_str(tcontext, tcontext); >> + __assign_str(scontext, scontext); >> + __assign_str(tclass, tclass); >> ), >> >> - TP_printk("tclass=%u audited=%x", >> - __entry->tclass, >> - __entry->audited) >> + TP_printk("requested=0x%x denied=0x%x audited=0x%x result=%d ssid=%u tsid=%u scontext=%s tcontext=%s tclass=%s", >> + __entry->requested, __entry->denied, __entry->audited, __entry->result, >> + __entry->ssid, __entry->tsid, __get_str(scontext), __get_str(tcontext), >> + __get_str(tclass) >> + ) >> ); >> >> #endif >> diff --git a/security/selinux/avc.c b/security/selinux/avc.c >> index b0a0af778b70..7de5cc5169af 100644 >> --- a/security/selinux/avc.c >> +++ b/security/selinux/avc.c >> @@ -705,35 +705,39 @@ static void avc_audit_post_callback(struct audit_buffer *ab, void *a) >> { >> struct common_audit_data *ad = a; >> struct selinux_audit_data *sad = ad->selinux_audit_data; >> - char *scontext; >> + char *scontext = NULL; >> + char *tcontext = NULL; >> + const char *tclass = NULL; >> u32 scontext_len; >> + u32 tcontext_len; >> int rc; >> >> - trace_selinux_audited(sad); >> - >> 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); >> + rc = security_sid_to_context(sad->state, sad->tsid, &tcontext, >> + &tcontext_len); >> if (rc) >> audit_log_format(ab, " tsid=%d", sad->tsid); >> else { >> - audit_log_format(ab, " tcontext=%s", scontext); >> - kfree(scontext); >> + audit_log_format(ab, " tcontext=%s", tcontext); >> } >> >> - audit_log_format(ab, " tclass=%s", secclass_map[sad->tclass-1].name); >> + tclass = secclass_map[sad->tclass-1].name; >> + audit_log_format(ab, " tclass=%s", tclass); >> >> if (sad->denied) >> audit_log_format(ab, " permissive=%u", sad->result ? 0 : 1); >> >> + trace_selinux_audited(sad, scontext, tcontext, tclass); >> + kfree(tcontext); >> + kfree(scontext); >> + >> /* in case of invalid context report also the actual context string */ >> rc = security_sid_to_context_inval(sad->state, sad->ssid, &scontext, >> &scontext_len);
On 8/13/20 11:35 AM, peter enderborg wrote: > On 8/13/20 5:05 PM, Casey Schaufler wrote: >> On 8/13/2020 7:48 AM, Thiébaud Weksteen wrote: >>> From: Peter Enderborg <peter.enderborg@sony.com> >>> >>> This patch adds further attributes to the event. These attributes are >>> helpful to understand the context of the message and can be used >>> to filter the events. >>> >>> There are three common items. Source context, target context and tclass. >>> There are also items from the outcome of operation performed. >>> >>> An event is similar to: >>> <...>-1309 [002] .... 6346.691689: selinux_audited: >>> requested=0x4000000 denied=0x4000000 audited=0x4000000 >>> result=-13 ssid=315 tsid=61 >> It may not be my place to ask, but *please please please* don't >> externalize secids. I understand that it's easier to type "42" >> than "system_r:cupsd_t:s0-s0:c0.c1023", and that it's easier for >> your tools to parse and store the number. Once you start training >> people that system_r:cupsd_t:s0-s0:c0.c1023 is secid 42 you'll >> never be able to change it. The secid will start showing up in >> scripts. Bad Things Will Happen. > Ok, it seems to mostly against having this performance options. > Yes, it is a kernel internal data. So is most of the kernel tracing. > I see it is a primary tool for kernel debugging but than can also be > used for user-space debugging tools. Hiding data for debuggers > does not make any sense too me. To be clear, userspace tools can't use fixed secid values because secids are dynamically assigned by SELinux and thus secid 42 need not correspond to the same security context across different boots even with the same kernel and policy. I wouldn't include them in the event unless it is common practice to include fields that can only be interpreted if you can debug the running kernel. It would be akin to including kernel pointers in the event (albeit without the KASLR ramifications).
On 8/13/20 5:49 PM, Stephen Smalley wrote: > On 8/13/20 11:35 AM, peter enderborg wrote: > >> On 8/13/20 5:05 PM, Casey Schaufler wrote: >>> On 8/13/2020 7:48 AM, Thiébaud Weksteen wrote: >>>> From: Peter Enderborg <peter.enderborg@sony.com> >>>> >>>> This patch adds further attributes to the event. These attributes are >>>> helpful to understand the context of the message and can be used >>>> to filter the events. >>>> >>>> There are three common items. Source context, target context and tclass. >>>> There are also items from the outcome of operation performed. >>>> >>>> An event is similar to: >>>> <...>-1309 [002] .... 6346.691689: selinux_audited: >>>> requested=0x4000000 denied=0x4000000 audited=0x4000000 >>>> result=-13 ssid=315 tsid=61 >>> It may not be my place to ask, but *please please please* don't >>> externalize secids. I understand that it's easier to type "42" >>> than "system_r:cupsd_t:s0-s0:c0.c1023", and that it's easier for >>> your tools to parse and store the number. Once you start training >>> people that system_r:cupsd_t:s0-s0:c0.c1023 is secid 42 you'll >>> never be able to change it. The secid will start showing up in >>> scripts. Bad Things Will Happen. >> Ok, it seems to mostly against having this performance options. >> Yes, it is a kernel internal data. So is most of the kernel tracing. >> I see it is a primary tool for kernel debugging but than can also be >> used for user-space debugging tools. Hiding data for debuggers >> does not make any sense too me. > > To be clear, userspace tools can't use fixed secid values because secids are dynamically assigned by SELinux and thus secid 42 need not correspond to the same security context across different boots even with the same kernel and policy. I wouldn't include them in the event unless it is common practice to include fields that can only be interpreted if you can debug the running kernel. It would be akin to including kernel pointers in the event (albeit without the KASLR ramifications). > Yes of course. Trace debugging is about running kernel. Would i make more sense if the was a debugfs entry with the sid's? This filter are a reminisce of the same filter used not only to catch denials. Doing a string compare for all syscalls keep the cpu busy. I will do an update without it.
On 8/13/20 5:49 PM, Stephen Smalley wrote: > On 8/13/20 11:35 AM, peter enderborg wrote: > >> On 8/13/20 5:05 PM, Casey Schaufler wrote: >>> On 8/13/2020 7:48 AM, Thiébaud Weksteen wrote: >>>> From: Peter Enderborg <peter.enderborg@sony.com> >>>> >>>> This patch adds further attributes to the event. These attributes are >>>> helpful to understand the context of the message and can be used >>>> to filter the events. >>>> >>>> There are three common items. Source context, target context and tclass. >>>> There are also items from the outcome of operation performed. >>>> >>>> An event is similar to: >>>> <...>-1309 [002] .... 6346.691689: selinux_audited: >>>> requested=0x4000000 denied=0x4000000 audited=0x4000000 >>>> result=-13 ssid=315 tsid=61 >>> It may not be my place to ask, but *please please please* don't >>> externalize secids. I understand that it's easier to type "42" >>> than "system_r:cupsd_t:s0-s0:c0.c1023", and that it's easier for >>> your tools to parse and store the number. Once you start training >>> people that system_r:cupsd_t:s0-s0:c0.c1023 is secid 42 you'll >>> never be able to change it. The secid will start showing up in >>> scripts. Bad Things Will Happen. >> Ok, it seems to mostly against having this performance options. >> Yes, it is a kernel internal data. So is most of the kernel tracing. >> I see it is a primary tool for kernel debugging but than can also be >> used for user-space debugging tools. Hiding data for debuggers >> does not make any sense too me. > > To be clear, userspace tools can't use fixed secid values because secids are dynamically assigned by SELinux and thus secid 42 need not correspond to the same security context across different boots even with the same kernel and policy. I wouldn't include them in the event unless it is common practice to include fields that can only be interpreted if you can debug the running kernel. It would be akin to including kernel pointers in the event (albeit without the KASLR ramifications). > > Just as a reference on my fedora system; out of 1808 events 244 as a pointer print. I don't see that there is any obfuscating aka "%pK" as there is for logs.
On Thu, 13 Aug 2020 19:14:10 +0200 peter enderborg <peter.enderborg@sony.com> wrote: > > To be clear, userspace tools can't use fixed secid values because > > secids are dynamically assigned by SELinux and thus secid 42 need > > not correspond to the same security context across different boots > > even with the same kernel and policy. I wouldn't include them in > > the event unless it is common practice to include fields that can > > only be interpreted if you can debug the running kernel. It would > > be akin to including kernel pointers in the event (albeit without > > the KASLR ramifications). > > > > > Just as a reference on my fedora system; out of 1808 events 244 as a > pointer print. I don't see that there is any obfuscating aka "%pK" as > there is for logs. Which is a reason why tracefs is root only. The "%p" gets obfuscated when printed from the trace file by default now. But they are consistent (where the same pointer shows up as the same hash). It's used mainly to map together events. For example, if you print the address of a skb in the networking events, it's good to know what events reference the same skb, and the pointer is used for that. -- Steve
On 8/13/20 7:38 PM, Steven Rostedt wrote: > On Thu, 13 Aug 2020 19:14:10 +0200 > peter enderborg <peter.enderborg@sony.com> wrote: > >>> To be clear, userspace tools can't use fixed secid values because >>> secids are dynamically assigned by SELinux and thus secid 42 need >>> not correspond to the same security context across different boots >>> even with the same kernel and policy. I wouldn't include them in >>> the event unless it is common practice to include fields that can >>> only be interpreted if you can debug the running kernel. It would >>> be akin to including kernel pointers in the event (albeit without >>> the KASLR ramifications). >>> >>> >> Just as a reference on my fedora system; out of 1808 events 244 as a >> pointer print. I don't see that there is any obfuscating aka "%pK" as >> there is for logs. > Which is a reason why tracefs is root only. > > The "%p" gets obfuscated when printed from the trace file by default > now. But they are consistent (where the same pointer shows up as the > same hash). > > It's used mainly to map together events. For example, if you print the > address of a skb in the networking events, it's good to know what > events reference the same skb, and the pointer is used for that. So what is your opinion on ssid? I dont mind removing them now since people dont like it and the strong use-case is not strong (yet). Is there any problem to put getting them back later if useful? And then before the strings so the evaluation of filter first come on number before stings Or is there already some mechanism that optimize for that? > -- Steve
On Thu, 13 Aug 2020 20:18:55 +0200 peter enderborg <peter.enderborg@sony.com> wrote: > > The "%p" gets obfuscated when printed from the trace file by default > > now. But they are consistent (where the same pointer shows up as the > > same hash). > > > > It's used mainly to map together events. For example, if you print the > > address of a skb in the networking events, it's good to know what > > events reference the same skb, and the pointer is used for that. > > So what is your opinion on ssid? I dont mind removing them > now since people dont like it and the strong use-case is not > strong (yet). Is there any problem to put getting them back > later if useful? And then before the strings so the evaluation > of filter first come on number before stings Or is there already > some mechanism that optimize for that? It's up to the owner of the trace event. I only replied to why pointers in general are useful, but they are mostly just "ids" to map to other trace events. We have the libtraceevent that should be used for parsing raw trace events in binary form. The library (which currently lives in the kernel's tools/lib/traceeevnt directory) I'm trying to get to have its own home that distros can package. It should never be an issue adding another field to an event, as the library gives the tools the ability to find a field of an event regardless of where it is positioned, and also let the tools know if the field exists or not. If that's what you are asking. -- Steve
diff --git a/include/trace/events/avc.h b/include/trace/events/avc.h index 07c058a9bbcd..ac5ef2e1c2c5 100644 --- a/include/trace/events/avc.h +++ b/include/trace/events/avc.h @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0 */ /* - * Author: Thiébaud Weksteen <tweek@google.com> + * Authors: Thiébaud Weksteen <tweek@google.com> + * Peter Enderborg <Peter.Enderborg@sony.com> */ #undef TRACE_SYSTEM #define TRACE_SYSTEM avc @@ -12,23 +13,43 @@ TRACE_EVENT(selinux_audited, - TP_PROTO(struct selinux_audit_data *sad), + TP_PROTO(struct selinux_audit_data *sad, + char *scontext, + char *tcontext, + const char *tclass + ), - TP_ARGS(sad), + TP_ARGS(sad, scontext, tcontext, tclass), TP_STRUCT__entry( - __field(unsigned int, tclass) - __field(unsigned int, audited) + __field(u32, requested) + __field(u32, denied) + __field(u32, audited) + __field(int, result) + __string(scontext, scontext) + __string(tcontext, tcontext) + __string(tclass, tclass) + __field(u32, ssid) + __field(u32, tsid) ), TP_fast_assign( - __entry->tclass = sad->tclass; - __entry->audited = sad->audited; + __entry->requested = sad->requested; + __entry->denied = sad->denied; + __entry->audited = sad->audited; + __entry->result = sad->result; + __entry->ssid = sad->ssid; + __entry->tsid = sad->tsid; + __assign_str(tcontext, tcontext); + __assign_str(scontext, scontext); + __assign_str(tclass, tclass); ), - TP_printk("tclass=%u audited=%x", - __entry->tclass, - __entry->audited) + TP_printk("requested=0x%x denied=0x%x audited=0x%x result=%d ssid=%u tsid=%u scontext=%s tcontext=%s tclass=%s", + __entry->requested, __entry->denied, __entry->audited, __entry->result, + __entry->ssid, __entry->tsid, __get_str(scontext), __get_str(tcontext), + __get_str(tclass) + ) ); #endif diff --git a/security/selinux/avc.c b/security/selinux/avc.c index b0a0af778b70..7de5cc5169af 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -705,35 +705,39 @@ static void avc_audit_post_callback(struct audit_buffer *ab, void *a) { struct common_audit_data *ad = a; struct selinux_audit_data *sad = ad->selinux_audit_data; - char *scontext; + char *scontext = NULL; + char *tcontext = NULL; + const char *tclass = NULL; u32 scontext_len; + u32 tcontext_len; int rc; - trace_selinux_audited(sad); - 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); + rc = security_sid_to_context(sad->state, sad->tsid, &tcontext, + &tcontext_len); if (rc) audit_log_format(ab, " tsid=%d", sad->tsid); else { - audit_log_format(ab, " tcontext=%s", scontext); - kfree(scontext); + audit_log_format(ab, " tcontext=%s", tcontext); } - audit_log_format(ab, " tclass=%s", secclass_map[sad->tclass-1].name); + tclass = secclass_map[sad->tclass-1].name; + audit_log_format(ab, " tclass=%s", tclass); if (sad->denied) audit_log_format(ab, " permissive=%u", sad->result ? 0 : 1); + trace_selinux_audited(sad, scontext, tcontext, tclass); + kfree(tcontext); + kfree(scontext); + /* in case of invalid context report also the actual context string */ rc = security_sid_to_context_inval(sad->state, sad->ssid, &scontext, &scontext_len);