* Alexei Starovoitov <a...@fb.com> wrote:
> > One of the major advantages of having an in-kernel BPF sandbox is to never
> > crash the kernel - and allowing BPF programs to just randomly modify the
> > return value of kernel functions sounds immensely broken to me.
> >
> > (And yes, I realize that kprobes are used here as a vehicle, but the point
> > remains.)
>
> yeah. modifying arbitrary function return pushes bpf outside of
> its safety guarantees and in that sense doing the same
> override_return could be done from a kernel module if kernel
> provides the x64 side of the facility introduced by this patch.
> On the other side adding parts of this feature to the kernel only
> to be used by external kernel module is quite ugly too and not
> something that was ever done before.
> How about we restrict this bpf_override_return() only to the functions
> which callers expect to handle errors ?
> We can add something similar to NOKPROBE_SYMBOL(). Like
> ALLOW_RETURN_OVERRIDE() and on btrfs side mark the functions
> we're going to test with this feature.
>
> Then 'not crashing kernel' requirement will be preserved.
> btrfs or whatever else we will be testing with override_return
> will be functioning in 'stress test' mode and if bpf program
> is not careful and returns error all the time then one particular
> subsystem (like btrfs) will not be functional, but the kernel
> will not be crashing.
> Thoughts?
Yeah, that approach sounds much better to me: it should be fundamentally be
opt-in, and should be documented that it should not be possible to crash the
kernel via changing the return value.
I'd make it a bit clearer in the naming what the purpose of the annotation is:
for
example would BPF_ALLOW_ERROR_INJECTION() work for you guys? I.e. I think it
should generally be used to change actual integer error values - or at most
user
pointers, but not kernel pointers. Not enforced in a type safe manner, but the
naming should give enough hints?
Such return-injection BFR programs can still totally confuse user-space
obviously:
for example returning an IO error could corrupt application data - but that's
the
nature of such facilities and similar results could already be achieved via
ptrace
as well. But the result of a BPF program should never be _worse_ than ptrace,
in
terms of kernel integrity.
Note that with such a safety mechanism in place no kernel message has to be
generated either I suspect.
In any case, my NAK would be lifted with such an approach.
Thanks,
Ingo
