On Thu, Jan 28, 2021 at 04:18:24PM -0800, Andy Lutomirski wrote: > 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. Please read the explanation again. It's an inlined probe_kernel_read. > > > > > 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. If you don't care to debug please don't provide wrong guesses.
