| Message ID | 20191205102552.19407-1-jolsa@kernel.org |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [PATCHv2] bpf: Emit audit messages upon successful prog load and unload | expand |
On Thu, Dec 5, 2019 at 5:26 AM Jiri Olsa <jolsa@kernel.org> wrote: > From: Daniel Borkmann <daniel@iogearbox.net> > > Allow for audit messages to be emitted upon BPF program load and > unload for having a timeline of events. The load itself is in > syscall context, so additional info about the process initiating > the BPF prog creation can be logged and later directly correlated > to the unload event. > > The only info really needed from BPF side is the globally unique > prog ID where then audit user space tooling can query / dump all > info needed about the specific BPF program right upon load event > and enrich the record, thus these changes needed here can be kept > small and non-intrusive to the core. > > Raw example output: > > # auditctl -D > # auditctl -a always,exit -F arch=x86_64 -S bpf > # ausearch --start recent -m 1334 > ... > ---- > time->Wed Nov 27 16:04:13 2019 > type=PROCTITLE msg=audit(1574867053.120:84664): proctitle="./bpf" > type=SYSCALL msg=audit(1574867053.120:84664): arch=c000003e syscall=321 \ > success=yes exit=3 a0=5 a1=7ffea484fbe0 a2=70 a3=0 items=0 ppid=7477 \ > pid=12698 auid=1001 uid=1001 gid=1001 euid=1001 suid=1001 fsuid=1001 \ > egid=1001 sgid=1001 fsgid=1001 tty=pts2 ses=4 comm="bpf" \ > exe="/home/jolsa/auditd/audit-testsuite/tests/bpf/bpf" \ > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null) > type=UNKNOWN[1334] msg=audit(1574867053.120:84664): prog-id=76 op=LOAD > ---- > time->Wed Nov 27 16:04:13 2019 > type=UNKNOWN[1334] msg=audit(1574867053.120:84665): prog-id=76 op=UNLOAD > ... > > Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> > Co-developed-by: Jiri Olsa <jolsa@kernel.org> > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > include/uapi/linux/audit.h | 1 + > kernel/bpf/syscall.c | 33 +++++++++++++++++++++++++++++++++ > 2 files changed, 34 insertions(+) > > v2 changes: > addressed Paul's comments from audit side: > - change 'event' field to 'op' > - change audit context passing > - check on 'op' value is within the limit > > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h > index c89c6495983d..32a5db900f47 100644 > --- a/include/uapi/linux/audit.h > +++ b/include/uapi/linux/audit.h > @@ -116,6 +116,7 @@ > #define AUDIT_FANOTIFY 1331 /* Fanotify access decision */ > #define AUDIT_TIME_INJOFFSET 1332 /* Timekeeping offset injected */ > #define AUDIT_TIME_ADJNTPVAL 1333 /* NTP value adjustment */ > +#define AUDIT_BPF 1334 /* BPF subsystem */ > > #define AUDIT_AVC 1400 /* SE Linux avc denial or grant */ > #define AUDIT_SELINUX_ERR 1401 /* Internal SE Linux Errors */ > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > index e3461ec59570..6536665f562c 100644 > --- a/kernel/bpf/syscall.c > +++ b/kernel/bpf/syscall.c > @@ -23,6 +23,7 @@ > #include <linux/timekeeping.h> > #include <linux/ctype.h> > #include <linux/nospec.h> > +#include <linux/audit.h> > #include <uapi/linux/btf.h> > > #define IS_FD_ARRAY(map) ((map)->map_type == BPF_MAP_TYPE_PERF_EVENT_ARRAY || \ > @@ -1306,6 +1307,36 @@ static int find_prog_type(enum bpf_prog_type type, struct bpf_prog *prog) > return 0; > } > > +enum bpf_audit { > + BPF_AUDIT_LOAD, > + BPF_AUDIT_UNLOAD, > + BPF_AUDIT_MAX, > +}; > + > +static const char * const bpf_audit_str[BPF_AUDIT_MAX] = { > + [BPF_AUDIT_LOAD] = "LOAD", > + [BPF_AUDIT_UNLOAD] = "UNLOAD", > +}; > + > +static void bpf_audit_prog(const struct bpf_prog *prog, unsigned int op) > +{ > + struct audit_context *ctx = NULL; > + struct audit_buffer *ab; > + > + if (audit_enabled == AUDIT_OFF) > + return; > + if (WARN_ON_ONCE(op >= BPF_AUDIT_MAX)) > + return; I feel bad saying this given the number of revisions we are at with this patch, but since we aren't even at -rc1 yet (although it will be here soon), I'm going to mention it anyway ;) ... if we move the "op >= BPF_AUDIT_MAX" above the audit_enabled check we will catch problems sooner in development, which is a very good thing as far as I'm concerned. Other than that, this looks good to me, and I see Steve has already given the userspace portion a thumbs-up. Have you started on the audit-testsuite test for this yet? > + if (op == BPF_AUDIT_LOAD) > + ctx = audit_context(); > + ab = audit_log_start(ctx, GFP_ATOMIC, AUDIT_BPF); > + if (unlikely(!ab)) > + return; > + audit_log_format(ab, "prog-id=%u op=%s", > + prog->aux->id, bpf_audit_str[op]); > + audit_log_end(ab); > +} > + > int __bpf_prog_charge(struct user_struct *user, u32 pages) > { > unsigned long memlock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT; > @@ -1421,6 +1452,7 @@ static void __bpf_prog_put(struct bpf_prog *prog, bool do_idr_lock) > { > if (atomic64_dec_and_test(&prog->aux->refcnt)) { > perf_event_bpf_event(prog, PERF_BPF_EVENT_PROG_UNLOAD, 0); > + bpf_audit_prog(prog, BPF_AUDIT_UNLOAD); > /* bpf_prog_free_id() must be called first */ > bpf_prog_free_id(prog, do_idr_lock); > __bpf_prog_put_noref(prog, true); > @@ -1830,6 +1862,7 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr) > */ > bpf_prog_kallsyms_add(prog); > perf_event_bpf_event(prog, PERF_BPF_EVENT_PROG_LOAD, 0); > + bpf_audit_prog(prog, BPF_AUDIT_LOAD); > > err = bpf_prog_new_fd(prog); > if (err < 0) > -- > 2.21.0
On Fri, Dec 06, 2019 at 04:11:13PM -0500, Paul Moore wrote: SNIP > > > > #define IS_FD_ARRAY(map) ((map)->map_type == BPF_MAP_TYPE_PERF_EVENT_ARRAY || \ > > @@ -1306,6 +1307,36 @@ static int find_prog_type(enum bpf_prog_type type, struct bpf_prog *prog) > > return 0; > > } > > > > +enum bpf_audit { > > + BPF_AUDIT_LOAD, > > + BPF_AUDIT_UNLOAD, > > + BPF_AUDIT_MAX, > > +}; > > + > > +static const char * const bpf_audit_str[BPF_AUDIT_MAX] = { > > + [BPF_AUDIT_LOAD] = "LOAD", > > + [BPF_AUDIT_UNLOAD] = "UNLOAD", > > +}; > > + > > +static void bpf_audit_prog(const struct bpf_prog *prog, unsigned int op) > > +{ > > + struct audit_context *ctx = NULL; > > + struct audit_buffer *ab; > > + > > + if (audit_enabled == AUDIT_OFF) > > + return; > > + if (WARN_ON_ONCE(op >= BPF_AUDIT_MAX)) > > + return; > > I feel bad saying this given the number of revisions we are at with > this patch, but since we aren't even at -rc1 yet (although it will be > here soon), I'm going to mention it anyway ;) > > ... if we move the "op >= BPF_AUDIT_MAX" above the audit_enabled check > we will catch problems sooner in development, which is a very good > thing as far as I'm concerned. sure, np will post v3 > > Other than that, this looks good to me, and I see Steve has already > given the userspace portion a thumbs-up. Have you started on the > audit-testsuite test for this yet? yep, it's ready.. waiting for kernel change ;-) https://github.com/olsajiri/audit-testsuite/commit/16888ea7f14fa0269feef623d2a96f15f9ea71c9 jirka
On Fri, Dec 6, 2019 at 4:28 PM Jiri Olsa <jolsa@redhat.com> wrote: > On Fri, Dec 06, 2019 at 04:11:13PM -0500, Paul Moore wrote: > > Other than that, this looks good to me, and I see Steve has already > > given the userspace portion a thumbs-up. Have you started on the > > audit-testsuite test for this yet? > > yep, it's ready.. waiting for kernel change ;-) > https://github.com/olsajiri/audit-testsuite/commit/16888ea7f14fa0269feef623d2a96f15f9ea71c9 Seeing tests for new features always makes me happy :)
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h index c89c6495983d..32a5db900f47 100644 --- a/include/uapi/linux/audit.h +++ b/include/uapi/linux/audit.h @@ -116,6 +116,7 @@ #define AUDIT_FANOTIFY 1331 /* Fanotify access decision */ #define AUDIT_TIME_INJOFFSET 1332 /* Timekeeping offset injected */ #define AUDIT_TIME_ADJNTPVAL 1333 /* NTP value adjustment */ +#define AUDIT_BPF 1334 /* BPF subsystem */ #define AUDIT_AVC 1400 /* SE Linux avc denial or grant */ #define AUDIT_SELINUX_ERR 1401 /* Internal SE Linux Errors */ diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index e3461ec59570..6536665f562c 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -23,6 +23,7 @@ #include <linux/timekeeping.h> #include <linux/ctype.h> #include <linux/nospec.h> +#include <linux/audit.h> #include <uapi/linux/btf.h> #define IS_FD_ARRAY(map) ((map)->map_type == BPF_MAP_TYPE_PERF_EVENT_ARRAY || \ @@ -1306,6 +1307,36 @@ static int find_prog_type(enum bpf_prog_type type, struct bpf_prog *prog) return 0; } +enum bpf_audit { + BPF_AUDIT_LOAD, + BPF_AUDIT_UNLOAD, + BPF_AUDIT_MAX, +}; + +static const char * const bpf_audit_str[BPF_AUDIT_MAX] = { + [BPF_AUDIT_LOAD] = "LOAD", + [BPF_AUDIT_UNLOAD] = "UNLOAD", +}; + +static void bpf_audit_prog(const struct bpf_prog *prog, unsigned int op) +{ + struct audit_context *ctx = NULL; + struct audit_buffer *ab; + + if (audit_enabled == AUDIT_OFF) + return; + if (WARN_ON_ONCE(op >= BPF_AUDIT_MAX)) + return; + if (op == BPF_AUDIT_LOAD) + ctx = audit_context(); + ab = audit_log_start(ctx, GFP_ATOMIC, AUDIT_BPF); + if (unlikely(!ab)) + return; + audit_log_format(ab, "prog-id=%u op=%s", + prog->aux->id, bpf_audit_str[op]); + audit_log_end(ab); +} + int __bpf_prog_charge(struct user_struct *user, u32 pages) { unsigned long memlock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT; @@ -1421,6 +1452,7 @@ static void __bpf_prog_put(struct bpf_prog *prog, bool do_idr_lock) { if (atomic64_dec_and_test(&prog->aux->refcnt)) { perf_event_bpf_event(prog, PERF_BPF_EVENT_PROG_UNLOAD, 0); + bpf_audit_prog(prog, BPF_AUDIT_UNLOAD); /* bpf_prog_free_id() must be called first */ bpf_prog_free_id(prog, do_idr_lock); __bpf_prog_put_noref(prog, true); @@ -1830,6 +1862,7 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr) */ bpf_prog_kallsyms_add(prog); perf_event_bpf_event(prog, PERF_BPF_EVENT_PROG_LOAD, 0); + bpf_audit_prog(prog, BPF_AUDIT_LOAD); err = bpf_prog_new_fd(prog); if (err < 0)