Re: [PATCHv2] bpf: Emit audit messages upon successful prog load and unload

From: Jiri Olsa <jolsa@redhat.com>
To: Paul Moore <paul@paul-moore.com>
Cc: Jiri Olsa <jolsa@kernel.org>, Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	netdev@vger.kernel.org, bpf@vger.kernel.org,
	linux-audit@redhat.com, Andrii Nakryiko <andriin@fb.com>,
	Yonghong Song <yhs@fb.com>, Martin KaFai Lau <kafai@fb.com>,
	Jakub Kicinski <jakub.kicinski@netronome.com>,
	Steve Grubb <sgrubb@redhat.com>, David Miller <davem@redhat.com>,
	Eric Paris <eparis@redhat.com>, Jiri Benc <jbenc@redhat.com>
Subject: Re: [PATCHv2] bpf: Emit audit messages upon successful prog load and unload
Date: Fri, 6 Dec 2019 22:27:46 +0100	[thread overview]
Message-ID: <20191206212746.GA30691@krava> (raw)
In-Reply-To: <CAHC9VhTWnNvfMAPz-WhD9Wqv6UZZDBdMxF9VuS3UeTLHLtfhHw@mail.gmail.com>

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