| Message ID | dda18ffd-0406-ec54-1014-b7d89a1bcd56@amazon.com |
|---|---|
| Headers | show |
| Series | Fix bpf: fix userspace access for bpf_probe_read{,str}() | expand |
On Wed, Apr 21, 2021 at 04:05:32PM +0300, Zidenberg, Tsahi wrote: > In arm64, kernelspace address accessors cannot be used to access > userspace addresses, which means bpf_probe_read{,str}() cannot access > userspace addresses. That causes e.g. command-line parameters to not > appear when snooping execve using bpf. Again, this really feels like a new feature, not a regression or bugfix at all. And in looking at these patches, that feeling really gets stronger. > This patch series takes the upstream solution. This solution also > changes user API in the following ways: > * Add probe_read_{user, kernel}{,_str} bpf helpers > * Add skb_output helper to the enum only (calling it not supported) > * Add support for %pks, %pus specifiers > > An alternative fix only takes the required logic to existing API without > adding new API, was suggested here: > https://www.spinics.net/lists/stable/msg454945.html > > Another option is to only take patches [1-4] of this patchset, and add > on top of them commit 8d92db5c04d1 ("bpf: rework the compat kernel probe > handling"). In that case, the last patch would require function renames > and conflict resolutions that were avoided in this patchset by pulling > patches [5-7]. The other option is to move your system to a newer kernel release that has this new feature, right? :) What prevents you from doing that today? What bug is this solving that worked in previous kernel releases and was broken in 5.4.y? thanks, greg k-h
On Fri, Apr 23, 2021 at 05:08:03PM +0200, Greg KH wrote: > On Wed, Apr 21, 2021 at 04:05:32PM +0300, Zidenberg, Tsahi wrote: > > In arm64, kernelspace address accessors cannot be used to access > > userspace addresses, which means bpf_probe_read{,str}() cannot access > > userspace addresses. That causes e.g. command-line parameters to not > > appear when snooping execve using bpf. > > Again, this really feels like a new feature, not a regression or bugfix > at all. And in looking at these patches, that feeling really gets > stronger. > > > This patch series takes the upstream solution. This solution also > > changes user API in the following ways: > > * Add probe_read_{user, kernel}{,_str} bpf helpers > > * Add skb_output helper to the enum only (calling it not supported) > > * Add support for %pks, %pus specifiers > > > > An alternative fix only takes the required logic to existing API without > > adding new API, was suggested here: > > https://www.spinics.net/lists/stable/msg454945.html > > > > Another option is to only take patches [1-4] of this patchset, and add > > on top of them commit 8d92db5c04d1 ("bpf: rework the compat kernel probe > > handling"). In that case, the last patch would require function renames > > and conflict resolutions that were avoided in this patchset by pulling > > patches [5-7]. > > The other option is to move your system to a newer kernel release that > has this new feature, right? :) > > What prevents you from doing that today? What bug is this solving that > worked in previous kernel releases and was broken in 5.4.y? And again, "feature parity across CPU architectures for the same release" is nothing that Linux has EVER guaranteed... thanks, greg k-h
In arm64, kernelspace address accessors cannot be used to access userspace addresses, which means bpf_probe_read{,str}() cannot access userspace addresses. That causes e.g. command-line parameters to not appear when snooping execve using bpf. This patch series takes the upstream solution. This solution also changes user API in the following ways: * Add probe_read_{user, kernel}{,_str} bpf helpers * Add skb_output helper to the enum only (calling it not supported) * Add support for %pks, %pus specifiers An alternative fix only takes the required logic to existing API without adding new API, was suggested here: https://www.spinics.net/lists/stable/msg454945.html Another option is to only take patches [1-4] of this patchset, and add on top of them commit 8d92db5c04d1 ("bpf: rework the compat kernel probe handling"). In that case, the last patch would require function renames and conflict resolutions that were avoided in this patchset by pulling patches [5-7]. Christoph Hellwig (3): 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 (4): 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 bpf: Restrict bpf_trace_printk()'s %s usage and add %pks, %pus specifier Petr Mladek (1): powerpc/bpf: Enable bpf_probe_read{, str}() on powerpc again Documentation/core-api/printk-formats.rst | 14 + arch/arm/Kconfig | 1 + arch/arm64/Kconfig | 1 + arch/powerpc/Kconfig | 1 + arch/x86/Kconfig | 1 + arch/x86/mm/Makefile | 2 +- arch/x86/mm/maccess.c | 43 +++ include/linux/uaccess.h | 8 +- include/uapi/linux/bpf.h | 123 ++++++--- init/Kconfig | 3 + kernel/trace/bpf_trace.c | 302 ++++++++++++++++------ kernel/trace/trace_kprobe.c | 2 +- lib/vsprintf.c | 12 + mm/maccess.c | 48 +++- tools/include/uapi/linux/bpf.h | 116 ++++++--- 15 files changed, 512 insertions(+), 165 deletions(-) create mode 100644 arch/x86/mm/maccess.c