netdev - Re: [PATCH bpf-next v2 4/8] bpf: Add probe_read_{user,kernel} and probe_read_{user,kernel}

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4BzYLjj0ExmO3hOV-JN+KJFs4vEW9PL7yDrK1+HTan+LVPQ@mail.gmail.com>
Date:   Thu, 31 Oct 2019 13:07:22 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     bpf <bpf@...r.kernel.org>, Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>
Subject: Re: [PATCH bpf-next v2 4/8] bpf: Add probe_read_{user,kernel} and
 probe_read_{user,kernel}_str helpers

On Wed, Oct 30, 2019 at 6:00 PM Daniel Borkmann <daniel@...earbox.net> wrote:
>
> The current bpf_probe_read() and bpf_probe_read_str() helpers are broken
> in that they assume they can be used for probing memory access for kernel
> space addresses /as well as/ user space addresses.
>
> However, plain use of probe_kernel_read() for both cases will attempt to
> always access kernel space address space given access is performed under
> KERNEL_DS and some archs in-fact have overlapping address spaces where a
> kernel pointer and user pointer would have the /same/ address value and
> therefore accessing application memory via bpf_probe_read{,_str}() would
> read garbage values.
>
> Lets fix BPF side by making use of recently added 3d7081822f7f ("uaccess:
> Add non-pagefault user-space read functions"). Unfortunately, the only way
> to fix this status quo is to add dedicated bpf_probe_read_{user,kernel}()
> and bpf_probe_read_{user,kernel}_str() helpers. The bpf_probe_read{,_str}()
> helpers are kept as-is to retain their current behavior.
>
> The two *_user() variants attempt the access always under USER_DS set, the
> two *_kernel() variants will -EFAULT when accessing user memory if the
> underlying architecture has non-overlapping address ranges, also avoiding
> throwing the kernel warning via 00c42373d397 ("x86-64: add warning for
> non-canonical user access address dereferences").
>
> Fixes: a5e8c07059d0 ("bpf: add bpf_probe_read_str helper")
> Fixes: 2541517c32be ("tracing, perf: Implement BPF programs attached to kprobes")
> Signed-off-by: Daniel Borkmann <daniel@...earbox.net>
> ---

LGTM!

Acked-by: Andrii Nakryiko <andriin@...com>

>  include/uapi/linux/bpf.h       | 120 +++++++++++++++-------
>  kernel/trace/bpf_trace.c       | 181 ++++++++++++++++++++++++---------
>  tools/include/uapi/linux/bpf.h | 120 +++++++++++++++-------
>  3 files changed, 297 insertions(+), 124 deletions(-)
>