Re: [PATCH v2 bpf-next 00/13] bpf: fmod_ret for each function that returns errors

[Anton Protopopov <a.s.protopopov@gmail.com>]

On 26/11/13 05:08AM, Alexei Starovoitov wrote:
> From: Alexei Starovoitov <ast@kernel.org>
> In short: with CONFIG_BPF_FMODRET_SCAN=y the build tags the functions that
> return errors, 12633 of them. A program with CAP_BPF and CAP_PERFMON can
> attach to one, skip a call and return one of its errors. It reads the
> arguments as untrusted pointers. No file of net, fs or mm changes. Test:
> one skipped call per function, 4401 functions of net, fs, mm. With this set
> 1 function in 180 then damages the kernel. A deny list by name can take
> those out.
> 
> 1. What for?
> BPF can keep user space away from a security bug in the rest of the kernel
> until the fixed kernel runs. An fmod_ret program attaches to a function on
> the way to the bug and fails the bad call:
>   SEC("fmod_ret/vfs_fallocate")    return len > MAX_LEN ? -EFBIG : 0;
>   err = vfs_fallocate(..) --> program
>                                returns 0      --> call it, err = its result
>                                returns -EFBIG --> skip it, err = -EFBIG
> Such a program needs targets in net, fs and mm. Today it has 1000 syscall
> entries, the security hooks and about 20 other functions. Of 417 fixes of
> one year that add a failing check to a function, the fixed function is a
> target: today 1 %, Anton's v3 (4) 17 %, this set 50 %.

The main reason for the difference is that my plugin was way more conservative
in "what is a clean path to error". However, given numbers, and how this is
supposed to be operated, more relaxed filter seems better. Authors of fmodret
programs are supposed to weight the consequences: forbid running a vulnerable
code vs. "leak resource", "lock one core", etc. Also, in many cases this should
be possible to fmodret upper the call stack, thus avoiding a problematic
attachment.

We can have a bit more targets via functions returning ERR_PTR(-errno), plus
maybe stuff like "if (err == -EINVAL) goto out", but maybe it was simpler
at IR (see 'bpf: Follow PHIs in bpf_scan_ir' in v3). All this is follow-ups.

For this series I don't see blockers, a few comments I had in individual patches.

One nit I've just remebered: why to tag functions which aren't attachable?
My old code stripped them out, they only take space and will be rejected
in any case.

I've also tested v2 on my x86.

> 2. What is a target?
> [...]