kprobes string reading broken on s390

From: Sven Schnelle
Date: Fri Jun 05 2020 - 07:06:38 EST


Hi Christoph,

with the latest linux-next i noticed that some tests in the
ftrace test suites are failing on s390, namely:

[FAIL] Kprobe event symbol argument
[FAIL] Kprobe event with comm arguments

The following doesn't work anymore:

cd /sys/kernel/tracing
echo 'p:testprobe _do_fork comm=$comm ' >kprobe_events
echo 1 >/sys/kernel/tracing/events/kprobes/testprobe/enable
cat /sys/kernel/tracing/trace

it will just show

test.sh-519 [012] .... 18.580625: testprobe: (_do_fork+0x0/0x3c8) comm=(fault)

Looking at d411a9c4e95a ("tracing/kprobes: handle mixed kernel/userspace probes
better") i see that there are two helpers for reading strings:

fetch_store_string_user() -> read string from user space
fetch_store_string() -> read string from kernel space(?)

but in the end both are using strncpy_from_user_nofault(), but i would
think that fetch_store_string() should use strncpy_from_kernel_nofault().
However, i'm not sure about the exact semantics of fetch_store_string(),
as there where a lot of wrong assumptions in the past, especially since
on x86 you usually don't fail if you use the same function for accessing kernel
and userspace although it's technically wrong.

Regards,
Sven

commit 81408eab8fcc79dc0871a95462b13176d3446f5e
Author: Sven Schnelle <svens@xxxxxxxxxxxxx>
Date: Fri Jun 5 13:01:24 2020 +0200

kprobes: use strncpy_from_kernel_nofault() in fetch_store_string()

Signed-off-by: Sven Schnelle <svens@xxxxxxxxxxxxx>

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index b1f21d558e45..ea8d0b094f1b 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1278,7 +1278,7 @@ fetch_store_string(unsigned long addr, void *dest, void *base)
* Try to get string again, since the string can be changed while
* probing.
*/
- ret = strncpy_from_user_nofault(__dest, (void *)addr, maxlen);
+ ret = strncpy_from_kernel_nofault(__dest, (void *)addr, maxlen);
if (ret >= 0)
*(u32 *)dest = make_data_loc(ret, __dest - base);