Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Alexei Starovoitov
Date: Fri Feb 22 2019 - 18:56:27 EST
On Fri, Feb 22, 2019 at 03:16:35PM -0800, Linus Torvalds wrote:
>
> So a kernel pointer value of 0x12345678 could be a value kernel
> pointer pointing to some random kmalloc'ed kernel memory, and a user
> pointer value of 0x12345678 could be a valid _user_ pointer pointing
> to some user mapping.
>
> See?
>
> If you access a user pointer, you need to use a user accessor function
> (eg "get_user()"), while if you access a kernel pointer you need to
> just dereference it directly (unless you can't trust it, in which case
> you need to use a _different_ accessor function).
that was clear already.
Reading 0x12345678 via probe_kernel_read can return valid value
and via get_user() can return another valid value on _some_ architectures.
> The fact that user and kernel pointers happen to be distinct on x86-64
> (right now) is just a random implementation detail.
yes and my point that people already rely on this implementation detail.
Say we implement
int bpf_probe_read(void *val, void *unsafe_ptr)
{
if (probe_kernel_read(val, unsafe_ptr) == OK) {
return 0;
} else (get_user(val, unsafe_ptr) == OK) {
return 0;
} else {
*val = 0;
return -EFAULT;
}
}
It will preserve existing bpf_probe_read() behavior on x86.
If x86 implementation changes tomorrow then progs that read user
addresses may start failing randomly because first probe_kernel_read()
will be returning random values from kernel memory and that's no good,
but at least we won't be breaking them today, so we have time to
introduce bpf_user_read and bpf_kernel_read and folks have time to adopt them.
Imo that's much better than making current bpf_probe_read() fail
on user addresses today and not providing a non disruptive path forward.