| Message ID | 20230504072910.3742842-1-ruanjinjie@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 99a670b2069c725a7b50318aa681d9cae8f89325 |
| Headers | show |
| Series | [v3] riscv: fix kprobe __user string arg print fault issue | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Single patches do not need cover letters |
| conchuod/tree_selection | success | Guessed tree name to be for-next at HEAD c2d3c8441e3d |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 5 and now 5 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 13 this patch: 13 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | fail | Errors and warnings before: 61 this patch: 62 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 5 this patch: 5 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 7 lines checked |
| conchuod/source_inline | success | Was 0 now: 0 |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
Ping. On 2023/5/4 15:29, Ruan Jinjie wrote: > On riscv qemu platform, when add kprobe event on do_sys_open() to show > filename string arg, it just print fault as follow: > > echo 'p:myprobe do_sys_open dfd=$arg1 filename=+0($arg2):string flags=$arg3 > mode=$arg4' > kprobe_events > > bash-166 [000] ...1. 360.195367: myprobe: (do_sys_open+0x0/0x84) > dfd=0xffffffffffffff9c filename=(fault) flags=0x8241 mode=0x1b6 > > bash-166 [000] ...1. 360.219369: myprobe: (do_sys_open+0x0/0x84) > dfd=0xffffffffffffff9c filename=(fault) flags=0x8241 mode=0x1b6 > > bash-191 [000] ...1. 360.378827: myprobe: (do_sys_open+0x0/0x84) > dfd=0xffffffffffffff9c filename=(fault) flags=0x98800 mode=0x0 > > As riscv do not select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE, > the +0($arg2) addr is processed as a kernel address though it is a > userspace address, cause the above filename=(fault) print. So select > ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE to avoid the issue, after that the > kprobe trace is ok as below: > > bash-166 [000] ...1. 96.767641: myprobe: (do_sys_open+0x0/0x84) > dfd=0xffffffffffffff9c filename="/dev/null" flags=0x8241 mode=0x1b6 > > bash-166 [000] ...1. 96.793751: myprobe: (do_sys_open+0x0/0x84) > dfd=0xffffffffffffff9c filename="/dev/null" flags=0x8241 mode=0x1b6 > > bash-177 [000] ...1. 96.962354: myprobe: (do_sys_open+0x0/0x84) > dfd=0xffffffffffffff9c filename="/sys/kernel/debug/tracing/events/kprobes/" > flags=0x98800 mode=0x0 > > Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> > Acked-by: Björn Töpel <bjorn@rivosinc.com> > --- > v2: > - add the config in alphabetical order > --- > v3: > - change signed-off-by to use full name > --- > arch/riscv/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index d2967fefa1d7..b08ed929e220 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -26,6 +26,7 @@ config RISCV > select ARCH_HAS_GIGANTIC_PAGE > select ARCH_HAS_KCOV > select ARCH_HAS_MMIOWB > + select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE > select ARCH_HAS_PMEM_API > select ARCH_HAS_PTE_SPECIAL > select ARCH_HAS_SET_DIRECT_MAP if MMU
Ruan Jinjie <ruanjinjie@huawei.com> writes:
> Ping.
Maybe it's not clear *why* we need
ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE on RISC-V?
ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE is used in the kernel to
determine what "memory access" function to use, e.g.
| #ifdef CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
| if ((unsigned long)str_val < TASK_SIZE)
| ret = strncpy_from_user_nofault(str_field, str_val, STR_VAR_LEN_MAX);
| else
| #endif
| ret = strncpy_from_kernel_nofault(str_field, str_val, STR_VAR_LEN_MAX);
RISC-V makes use of the SUM bit [1], which requires the kernel flips a
bit explicitly to touch user memory, so it's important to use the
correct access function.
What this means, is that if
CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE is *not* defined for RV
kernels, the copy_from_kernel will fault (as Jinjie points out in the
commit message).
The fixes tag should be when Daniel Borkmann introduced the config -- or
that makes sense from a backport perspective.
Maybe Palmer can add a lore link to this post, and the following
fixes-tag
Fixes: 0ebeea8ca8a4 ("bpf: Restrict bpf_probe_read{, str}() only to archs where they work")
when applying?
Björn
[1] https://github.com/riscv/riscv-isa-manual/blob/main/src/supervisor.adoc?plain=1#L118
On Thu, 04 May 2023 15:29:10 +0800, Ruan Jinjie wrote: > On riscv qemu platform, when add kprobe event on do_sys_open() to show > filename string arg, it just print fault as follow: > > echo 'p:myprobe do_sys_open dfd=$arg1 filename=+0($arg2):string flags=$arg3 > mode=$arg4' > kprobe_events > > bash-166 [000] ...1. 360.195367: myprobe: (do_sys_open+0x0/0x84) > dfd=0xffffffffffffff9c filename=(fault) flags=0x8241 mode=0x1b6 > > [...] Applied, thanks! [1/1] riscv: fix kprobe __user string arg print fault issue https://git.kernel.org/palmer/c/99a670b2069c Best regards,
Hello: This patch was applied to riscv/linux.git (fixes) by Palmer Dabbelt <palmer@rivosinc.com>: On Thu, 4 May 2023 15:29:10 +0800 you wrote: > On riscv qemu platform, when add kprobe event on do_sys_open() to show > filename string arg, it just print fault as follow: > > echo 'p:myprobe do_sys_open dfd=$arg1 filename=+0($arg2):string flags=$arg3 > mode=$arg4' > kprobe_events > > bash-166 [000] ...1. 360.195367: myprobe: (do_sys_open+0x0/0x84) > dfd=0xffffffffffffff9c filename=(fault) flags=0x8241 mode=0x1b6 > > [...] Here is the summary with links: - [v3] riscv: fix kprobe __user string arg print fault issue https://git.kernel.org/riscv/c/99a670b2069c You are awesome, thank you!
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index d2967fefa1d7..b08ed929e220 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -26,6 +26,7 @@ config RISCV select ARCH_HAS_GIGANTIC_PAGE select ARCH_HAS_KCOV select ARCH_HAS_MMIOWB + select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE select ARCH_HAS_PMEM_API select ARCH_HAS_PTE_SPECIAL select ARCH_HAS_SET_DIRECT_MAP if MMU