Andrii Nakryiko wrote:

On Mon, Sep 27, 2021 at 11:20 AM John Fastabend
[off-list ref] wrote:

quoted

Dave Marchevsky wrote:

quoted

On 9/23/21 10:02 PM, Andrii Nakryiko wrote:

quoted

On Thu, Sep 23, 2021 at 6:27 PM Dave Marchevsky [off-list ref] wrote:

quoted

On 9/23/21 4:51 PM, Alexei Starovoitov wrote:

quoted

On Mon, Sep 20, 2021 at 08:11:10AM -0700, Dave Marchevsky wrote:

quoted

The verifier currently logs some useful statistics in
print_verification_stats. Although the text log is an effective feedback
tool for an engineer iterating on a single application, it would also be
useful to enable tracking these stats in a more structured form for
fleetwide or historical analysis, which this patchset attempts to do.

[...] 

quoted

quoted

Seems reasonable to me - and attaching a BPF program to the tracepoint to
grab data is delightfully meta :)

I'll do a pass on alternate implementation with _just_ tracepoint, no
prog_info or fdinfo, can add minimal or full stats to those later if
necessary.

We can also use a hook point here to enforce policy on allowing the
BPF program to load or not using the stats here. For now basic
insn is a good start to allow larger/smaller programs to be loaded,
but we might add other info like call bitmask, features, types, etc.
If one of the arguments is the bpf_attr struct we can just read
lots of useful program info out directly.

We would need something different from a tracepoint though to let
it return a reject|accept code. How about a new hook type that
has something similar to sockops that lets us just return an
accept or reject code?

By doing this we can check loader signatures here to be sure the
loader is signed or otherwise has correct permissions to be loading
whatever type of bpf program is here.

For signing and generally preventing some BPF programs from loading
(e.g., if there is some malicious BPF program that takes tons of
memory to be validated), wouldn't you want to check that before BPF
verifier spent all those resources on verification? So maybe there
will be another hook before BPF prog is validated for that? Basically,
if you don't trust any BPF program unless it is signed, I'd expect you
check signature before BPF verifier does its heavy job.

Agree, for basic sig check or anything that just wants to look at
the task_struct storage for some attributes before we verify is
more efficient. The only reason I suggested after is if we wanted
to start auditing/enforcing on calls or map read/writes, etc. these
we would need the verifier to help tabulate.

When I hacked it in for experimenting I put the hook in the sys
bpf load path before the verifier runs. That seemed to work for
the simpler sig check cases I was running.

OTOH though if we have a system with lots of BPF failed loads this
would indicate a more serious problem that an admin should fix
so might be nicer code-wise to just have a single hook after verifier
vs optimizing to two one in front and one after. 

quoted

Thanks,
John