[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190222235618.dxewmv5dukltaoxl@ast-mbp.dhcp.thefacebook.com>
Date: Fri, 22 Feb 2019 15:56:20 -0800
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: David Miller <davem@...emloft.net>,
Masami Hiramatsu <mhiramat@...nel.org>,
Steven Rostedt <rostedt@...dmis.org>,
Andy Lutomirski <luto@...capital.net>,
Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
Ingo Molnar <mingo@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
stable <stable@...r.kernel.org>,
Changbin Du <changbin.du@...il.com>,
Jann Horn <jannh@...gle.com>,
Kees Cook <keescook@...omium.org>,
Andrew Lutomirski <luto@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Netdev <netdev@...r.kernel.org>, bpf@...r.kernel.org
Subject: Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access
kernel memory that can fault
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.
Powered by blists - more mailing lists