On Thu, Jan 28, 2021 at 04:45:41PM -0800, Andy Lutomirski wrote:

On Thu, Jan 28, 2021 at 4:41 PM Alexei Starovoitov
[off-list ref] wrote:

quoted

On Thu, Jan 28, 2021 at 04:29:51PM -0800, Andy Lutomirski wrote:

quoted

BPF generated a NULL pointer dereference (where NULL is a user
pointer) and expected it to recover cleanly. What exactly am I
supposed to debug?  IMO the only thing wrong with the x86 code is that
it doesn't complain more loudly.  I will fix that, too.

are you saying that NULL is a _user_ pointer?!
It's NULL. All zeros.
probe_read_kernel(NULL) was returning EFAULT on it and should continue doing so.

probe_read_kernel() does not exist.  get_kernel_nofault() returns -ERANGE.

That was an old name. bpf_probe_read_kernel() is using copy_from_kernel_nofault() now.

And yes, NULL is a user pointer.  I can write you a little Linux
program that maps some real valid data at user address 0.  As I noted

are you sure? I thought mmap of addr zero was disallowed long ago.

when I first analyzed this bug, because NULL is a user address, bpf is
incorrectly triggering the *user* fault handling code, and that code
is objecting.

I propose the following fix to the x86 code.  I'll send it as a real
patch tomorrow.

https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/fixes&id=f61282777772f375bba7130ae39ccbd7e83878b2

You realize that you propose to panic kernels for all existing tracing users, right?

Do you have a specific security concern with treating fault on NULL special?