On Thu, Jan 28, 2021 at 4:11 PM Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > On Thu, Jan 28, 2021 at 03:51:13PM -0800, Andy Lutomirski wrote: > > > > Okay, so I guess you're trying to inline probe_read_kernel(). But > > that means you have to inline a valid implementation. In particular, > > you need to check that you're accessing *kernel* memory. Just like > > That check is on the verifier side. It only does it for kernel > pointers with known types. > In a sequnce a->b->c the verifier guarantees that 'a' is valid > kernel pointer and it's also !null. Then it guarantees that offsetof(b) > points to valid kernel field which is also a pointer. > What it doesn't check that b != null, so > that users don't have to write silly code with 'if (p)' after every > dereference. That sounds like a verifier and/or JIT bug. If you have a pointer p (doesn't matter whether p is what you call a or a->b) and you have not confirmed that p points to the kernel range, you may not generate a load from that pointer. > > > how get_user() validates that the pointer points into user memory, > > your helper should bounds check the pointer. On x86, you could check > > the high bit. > > > > As an extra complication, we should really add logic to > > get_kernel_nofault() to verify that the pointer points into actual > > memory as opposed to MMIO space (or future incoherent MKTME space or > > something like that, sigh). This will severely complicate inlining > > it. And we should *really* make the same fix to get_kernel_nofault() > > -- it should validate that the pointer is a kernel pointer. > > > > Is this really worth inlining instead of having the BPF JIT generate > > an out of line call to a real C function? That would let us put in a > > sane implementation. > > It's out of the question. > JIT cannot generate a helper call for single bpf insn without huge overhead. > All registers are used. It needs full save/restore, stack increase, etc. > > Anyhow I bet the bug we're discussing has nothing to do with bpf and jit. > Something got changed and now probe_kernel_read(NULL) warns on !SMAP. > This is something to debug. The bug is in bpf.