On Fri, Jul 24, 2020 at 5:15 AM Thiébaud Weksteen <tweek@xxxxxxxxxx> wrote: > > The audit data currently captures which process and which target > is responsible for a denial. There is no data on where exactly in the > process that call occurred. Debugging can be made easier by being able to > reconstruct the unified kernel and userland stack traces [1]. Add a > tracepoint on the SELinux denials which can then be used by userland > (i.e. perf). > > Although this patch could manually be added by each OS developer to > trouble shoot a denial, adding it to the kernel streamlines the > developers workflow. > > [1] https://source.android.com/devices/tech/debug/native_stack_dump > > Signed-off-by: Thiébaud Weksteen <tweek@xxxxxxxxxx> > Signed-off-by: Joel Fernandes <joelaf@xxxxxxxxxx> > --- > MAINTAINERS | 1 + > include/trace/events/selinux.h | 35 ++++++++++++++++++++++++++++++++++ > security/selinux/avc.c | 6 ++++++ > 3 files changed, 42 insertions(+) > create mode 100644 include/trace/events/selinux.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index e64cdde81851..6b6cd5e13537 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -15358,6 +15358,7 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git > F: Documentation/ABI/obsolete/sysfs-selinux-checkreqprot > F: Documentation/ABI/obsolete/sysfs-selinux-disable > F: Documentation/admin-guide/LSM/SELinux.rst > +F: include/trace/events/selinux.h > F: include/uapi/linux/selinux_netlink.h > F: scripts/selinux/ > F: security/selinux/ > diff --git a/include/trace/events/selinux.h b/include/trace/events/selinux.h > new file mode 100644 > index 000000000000..e247187a8135 > --- /dev/null > +++ b/include/trace/events/selinux.h > @@ -0,0 +1,35 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#undef TRACE_SYSTEM > +#define TRACE_SYSTEM selinux > + > +#if !defined(_TRACE_SELINUX_H) || defined(TRACE_HEADER_MULTI_READ) > +#define _TRACE_SELINUX_H > + > +#include <linux/ktime.h> > +#include <linux/tracepoint.h> > + > +TRACE_EVENT(selinux_denied, > + > + TP_PROTO(int cls, int av), > + > + TP_ARGS(cls, av), > + > + TP_STRUCT__entry( > + __field(int, cls) > + __field(int, av) > + ), > + > + TP_fast_assign( > + __entry->cls = cls; > + __entry->av = av; > + ), > + > + TP_printk("denied %d %d", > + __entry->cls, > + __entry->av) > +); I would think you would want to log av as %x for easier interpretation especially when there are multiple permissions being checked at once (which can happen). Also both cls and av would properly be unsigned values. Only other question I have is whether it would be beneficial to include other information here to help uniquely identify/correlate the denial with the avc: message and whether any decoding of the class, av, or other information could/should be done here versus in some userland helper.
