[RFC,stable,5.4,0/8] bpf: Fix bpf_probe_read/bpf_probe_read_str helpers

  hi,
we see broken access to user space with bpf_probe_read/bpf_probe_read_str
helpers on arm64 with 5.4 kernel. The problem is that both helpers try to
read user memory by calling probe_kernel_read, which seems to work on x86
but fails on arm64.

There are several fixes after v5.4 to address this issue.

There was an attempt to fix that in past [1], but it deviated far from
upstream changes.

This patchset tries to follow the upstream changes with 2 notable exceptions:

   1) bpf: Add probe_read_{user, kernel} and probe_read_{user, kernel}_str helpers

      - this upsgream patch adds new helpers, which we don't need to do,
        we just need following functions (and related helper's glue):

          bpf_probe_read_kernel_common
          bpf_probe_read_kernel_str_common

        that implement bpf_probe_read* helpers and receive fix in next
        patch below, ommiting any other hunks

   2) bpf: rework the compat kernel probe handling

      - taking only fixes for functions and realted helper's glue that we took
        from patch above, ommiting any other hunks


It's possible to add new helpers and keep the patches closer to upstream,
but I thought trying this way first as RFC without adding any new helpers
into stable kernel, which might possibly end up later with additional fixes.

Also I'm sending this as RFC, because I might be missing some mm related
dependency change, that I'm not aware of.

We tested new kernel with our use case on arm64 and x86.

thanks,
jirka


[1] https://yhbt.net/lore/all/YHGMFzQlHomDtZYG@kroah.com/t/
---
Andrii Nakryiko (1):
      bpf: bpf_probe_read_kernel_str() has to return amount of data read on success

Christoph Hellwig (4):
      maccess: clarify kerneldoc comments
      maccess: rename strncpy_from_unsafe_user to strncpy_from_user_nofault
      maccess: rename strncpy_from_unsafe_strict to strncpy_from_kernel_nofault
      bpf: rework the compat kernel probe handling

Daniel Borkmann (3):
      uaccess: Add strict non-pagefault kernel-space read function
      bpf: Add probe_read_{user, kernel} and probe_read_{user, kernel}_str helpers
      bpf: Restrict bpf_probe_read{, str}() only to archs where they work

 arch/arm/Kconfig            |   1 +
 arch/arm64/Kconfig          |   1 +
 arch/x86/Kconfig            |   1 +
 arch/x86/mm/Makefile        |   2 +-
 arch/x86/mm/maccess.c       |  43 ++++++++++++++++++++++++++++++++++++++
 include/linux/uaccess.h     |   8 +++++--
 init/Kconfig                |   3 +++
 kernel/trace/bpf_trace.c    | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------
 kernel/trace/trace_kprobe.c |   2 +-
 mm/maccess.c                |  63 ++++++++++++++++++++++++++++++++++++++++++++++++-------
 10 files changed, 207 insertions(+), 57 deletions(-)
 create mode 100644 arch/x86/mm/maccess.c
  

On Mon, May 22, 2023 at 10:33:44PM +0200, Jiri Olsa wrote:
> hi,
> we see broken access to user space with bpf_probe_read/bpf_probe_read_str
> helpers on arm64 with 5.4 kernel. The problem is that both helpers try to
> read user memory by calling probe_kernel_read, which seems to work on x86
> but fails on arm64.

Has this ever worked on arm64 for the 5.4 kernel tree?  If not, it's not
really a regression, and so, why not use a newer kernel that has this
new feature added to it there?

In other words, what requires you to use the 5.4.y tree and requires
feature parity across architectures?

thanks,

greg k-h
  

On Fri, May 26, 2023 at 07:54:17PM +0100, Greg KH wrote:
> On Mon, May 22, 2023 at 10:33:44PM +0200, Jiri Olsa wrote:
> > hi,
> > we see broken access to user space with bpf_probe_read/bpf_probe_read_str
> > helpers on arm64 with 5.4 kernel. The problem is that both helpers try to
> > read user memory by calling probe_kernel_read, which seems to work on x86
> > but fails on arm64.
> 
> Has this ever worked on arm64 for the 5.4 kernel tree?  If not, it's not
> really a regression, and so, why not use a newer kernel that has this
> new feature added to it there?
> 
> In other words, what requires you to use the 5.4.y tree and requires
> feature parity across architectures?

we have a customer running ok on x86 v5.4, but arm64 is broken with
the same bpf/user space code

upgrade is an option of course, but it's not a big change and we can
have 5.4 working on arm64 as well

I can send out the change that will be closer to upstream changes,
if that's a concern.. with adding the new probe helpers, which I
guess is not a problem, because it does not change current API

jirka
  

On Sun, May 28, 2023 at 10:02:49PM +0200, Jiri Olsa wrote:
> On Fri, May 26, 2023 at 07:54:17PM +0100, Greg KH wrote:
> > On Mon, May 22, 2023 at 10:33:44PM +0200, Jiri Olsa wrote:
> > > hi,
> > > we see broken access to user space with bpf_probe_read/bpf_probe_read_str
> > > helpers on arm64 with 5.4 kernel. The problem is that both helpers try to
> > > read user memory by calling probe_kernel_read, which seems to work on x86
> > > but fails on arm64.
> > 
> > Has this ever worked on arm64 for the 5.4 kernel tree?  If not, it's not
> > really a regression, and so, why not use a newer kernel that has this
> > new feature added to it there?
> > 
> > In other words, what requires you to use the 5.4.y tree and requires
> > feature parity across architectures?
> 
> we have a customer running ok on x86 v5.4, but arm64 is broken with
> the same bpf/user space code

Again why can they not use a newer kernel version?  What forces this
customer to be stuck with a specific kernel version that spans different
processor types?

> upgrade is an option of course, but it's not a big change and we can
> have 5.4 working on arm64 as well

For loads of other reasons, I'd recommend 5.15 or newer for arm64, so
why not use that?

> I can send out the change that will be closer to upstream changes,
> if that's a concern.. with adding the new probe helpers, which I
> guess is not a problem, because it does not change current API

You are trying to add features to a stable kernel that are not
regression fixes, which is something that we generally do not accept
into stable kernels.

thnaks,

greg k-h