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

From: Jiri Olsa <jolsa@redhat.com>
To: Steve Grubb <sgrubb@redhat.com>
Cc: Paul Moore <paul@paul-moore.com>, 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>,
	David Miller <davem@redhat.com>, Eric Paris <eparis@redhat.com>,
	Jiri Benc <jbenc@redhat.com>
Subject: Re: [RFC] bpf: Emit audit messages upon successful prog load and unload
Date: Tue, 3 Dec 2019 09:46:26 +0100	[thread overview]
Message-ID: <20191203084626.GB17468@krava> (raw)
In-Reply-To: <1915471.OmxkCOUsnW@x2>

On Mon, Dec 02, 2019 at 11:57:22PM -0500, Steve Grubb wrote:
> On Monday, December 2, 2019 6:00:14 PM EST Paul Moore wrote:
> > On Thu, Nov 28, 2019 at 4:16 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.

SNIP

> > I think you would probably also want to check the results of
> > audit_dummy_context() here as well, see all the various audit_XXX()
> > functions in include/linux/audit.h as an example.  You'll see a
> > pattern similar to the following:
> > 
> > static inline void audit_foo(...)
> > {
> >   if (unlikely(!audit_dummy_context()))
> >     __audit_foo(...)
> > }
> > 
> > > +       ab = audit_log_start(audit_context(), GFP_ATOMIC, AUDIT_BPF);
> > > +       if (unlikely(!ab))
> > > +               return;
> > > +       audit_log_format(ab, "prog-id=%u op=%s",
> > > +                        prog->aux->id, bpf_audit_str[op]);
> > 
> > Is it worth putting some checks in here to make sure that you don't
> > blow past the end of the bpf_audit_str array?
> 
> I am wondering if prog-id was really the only information that was needed? Is 
> it meaningful to other tools? Does that correlate to anything in /proc? In 
> earlier discussion, it sounded like more information was needed to be sure 
> what was happening.

yep, as David mentions in the changelog the global ID is enough,
because you can get all the other bpf program info based on that

jirka