* Re: [PATCH v2] tracing/probes: Zero out faulted dereferenced fields
2026-11-12 15:58 [PATCH v2] tracing/probes: Zero out faulted dereferenced fields Steven Rostedt
@ 2026-11-12 16:26 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-11-12 16:26 UTC (permalink / raw)
To: Steven Rostedt, Linux trace kernel
Thank you for your contribution! Sashiko AI review found 4 potential
issues to consider:
- [Medium] tracing/probes: stage 3 in process_fetch_insn_bottom()
dereferences NULL val
- [Medium] tracing/probes: FETCH_OP_CPU_PTR in
process_fetch_insn_bottom() shifts NULL
- [Medium] tracing/probes: FETCH_OP_LP_ARRAY in
process_fetch_insn_bottom() advances NULL
- [Low] tracing/probes: trace_uprobe probe_mem_read() leaves partial
bytes on fault
--
commit 0adbcc076a7e0ebaeea272396034e57fe028eb89
Author: Steven Rostedt <rostedt@goodmis.org>
tracing/probes: Zero out faulted dereferenced fields
This commit zeroes the destination buffer in probe_mem_read() and
probe_mem_read_user() on read failure, and updates stage 2 of
process_fetch_insn_bottom() to skip NULL pointer dereferences and continue
instead of returning on fault.
> diff --git a/kernel/trace/trace_probe_kernel.h b/kernel/trace/trace_probe_kernel.h
> index 4eb4acfa09ba1..00542a162d307 100644
> --- a/kernel/trace/trace_probe_kernel.h
> +++ b/kernel/trace/trace_probe_kernel.h
> @@ -102,18 +102,27 @@ static nokprobe_inline int
> probe_mem_read_user(void *dest, void *src, size_t size)
> {
> const void __user *uaddr = (__force const void __user *)src;
> + int ret;
>
> - return copy_from_user_nofault(dest, uaddr, size);
> + ret = copy_from_user_nofault(dest, uaddr, size);
> + if (ret < 0)
> + memset(dest, 0, size);
> + return ret;
> }
>
> static nokprobe_inline int
> probe_mem_read(void *dest, void *src, size_t size)
> {
> + int ret;
> +
> #ifdef CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
> if ((unsigned long)src < TASK_SIZE)
> return probe_mem_read_user(dest, src, size);
> #endif
> - return copy_from_kernel_nofault(dest, src, size);
> + ret = copy_from_kernel_nofault(dest, src, size);
> + if (ret < 0)
> + memset(dest, 0, size);
> + return ret;
> }
[Severity: Low]
Does probe_mem_read() in kernel/trace/trace_uprobe.c also need to zero dest
when copy_from_user() fails?
kernel/trace/trace_uprobe.c defines its own probe_mem_read() and includes
kernel/trace/trace_probe_tmpl.h:
kernel/trace/trace_uprobe.c:probe_mem_read() {
void __user *vaddr = (void __force __user *)src;
return copy_from_user(dest, vaddr, size) ? -EFAULT : 0;
}
On a partial fault across a page boundary, _inline_copy_from_user() only
zeroes the uncopied tail bytes, leaving the initial copied bytes in dest.
Because stage 2 in process_fetch_insn_bottom() now ignores the return value
of probe_mem_read(), an unaligned uprobe dereference that faults partway
through a pointer leaves a partially written val that subsequent stage 2 or
stage 3 operations continue to dereference or store.
> diff --git a/kernel/trace/trace_probe_tmpl.h b/kernel/trace/trace_probe_tmpl.h
> index 8db12f758fdae..210a75ed5a92f 100644
> --- a/kernel/trace/trace_probe_tmpl.h
> +++ b/kernel/trace/trace_probe_tmpl.h
> @@ -137,23 +137,24 @@ process_fetch_insn_bottom(struct fetch_insn *code, unsigned long val,
> lval = val;
> switch (code->op) {
> case FETCH_OP_DEREF:
> - ret = probe_mem_read(&val, (void *)val + code->offset,
> - sizeof(val));
> + /* Do not dereference NULL pointers */
> + if (val)
> + probe_mem_read(&val, (void *)val + code->offset,
> + sizeof(val));
> break;
> case FETCH_OP_UDEREF:
> - ret = probe_mem_read_user(&val,
> - (void *)val + code->offset, sizeof(val));
> + /* Do not dereference NULL pointers */
> + if (val)
> + probe_mem_read_user(&val, (void *)val + code->offset,
> + sizeof(val));
> break;
[Severity: Medium]
When val is 0 (or becomes 0 after a stage 2 fault), doesn't stage 3 still
dereference (void *)0 + code->offset?
finalize_fetch_insn() converts the final FETCH_OP_DEREF or FETCH_OP_UDEREF
of a dereference chain (such as ->pgd in prev->mm->pgd) into
FETCH_OP_ST_MEM, FETCH_OP_ST_UMEM, FETCH_OP_ST_STRING, or
FETCH_OP_ST_USTRING in stage 3, which do not check if (val):
kernel/trace/trace_probe_tmpl.h:process_fetch_insn_bottom() {
...
switch (code->op) {
case FETCH_OP_ST_RAW:
fetch_store_raw(val, code, dest);
break;
case FETCH_OP_ST_MEM:
probe_mem_read(dest, (void *)val + code->offset, code->size);
break;
case FETCH_OP_ST_UMEM:
probe_mem_read_user(dest, (void *)val + code->offset, code->size);
break;
case FETCH_OP_ST_STRING:
loc = *(u32 *)dest;
ret = fetch_store_string(val + code->offset, dest, base);
break;
...
}
When val is 0, stage 3 still attempts to read from (void *)0 + code->offset.
On architectures with CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE,
probe_mem_read(), fetch_store_strlen(), and fetch_store_string() see
(unsigned long)src < TASK_SIZE for positive offsets and redirect this NULL
kernel pointer dereference to user space via probe_mem_read_user() or
fetch_store_string_user(), which raises a page fault on every NULL pointer
event and copies unrelated user-space memory into the trace event if that
address is mapped in current. In addition, when a stage 2 dereference
preceding FETCH_OP_ST_SYMSTR faults, continuing to stage 3 with val == 0
passes 0 + code->offset to fetch_store_symstrlen() and
fetch_store_symstring(), formatting 0x0 instead of failing.
> case FETCH_OP_CPU_PTR:
> val = (unsigned long)this_cpu_ptr((void __percpu *)val);
> - ret = 0;
> break;
[Severity: Medium]
Should FETCH_OP_CPU_PTR also check if (val) before calling this_cpu_ptr()?
In expressions such as this_cpu_ptr(+0($arg1)) or this_cpu_read(+0($arg1)),
unwind_parse_states() emits FETCH_OP_DEREF followed by FETCH_OP_CPU_PTR.
Now that stage 2 continues with val == 0 when the preceding FETCH_OP_DEREF
reads NULL or faults, FETCH_OP_CPU_PTR computes this_cpu_ptr(NULL), turning
val into the non-zero per-CPU base offset (my_cpu_offset on SMP) so
subsequent instructions record or dereference my_cpu_offset + code->offset
instead of 0.
> default:
> lval = llval;
> goto out;
> }
> - if (ret)
> - return ret;
> llval = lval;
> code++;
> } while (1);
[Severity: Medium]
Can continuing past stage 2 when val is 0 cause FETCH_OP_LP_ARRAY to advance
from NULL into non-zero addresses on subsequent array elements?
kernel/trace/trace_probe_tmpl.h:process_fetch_insn_bottom() {
...
array:
/* the last stage: Loop on array */
if (code->op == FETCH_OP_LP_ARRAY) {
if (ret < 0)
ret = 0;
total += ret;
if (++i < code->param) {
code = s3;
if (s3->op != FETCH_OP_ST_STRING &&
s3->op != FETCH_OP_ST_USTRING) {
dest += s3->size;
val += s3->size;
goto stage3;
}
code--;
val = lval + sizeof(char *);
if (dest) {
dest += sizeof(u32);
*(u32 *)dest = update_data_loc(loc, ret);
}
goto stage2;
}
...
}
When the base pointer of an array argument is NULL or a stage 2 dereference
faults, execution now reaches array: with val == 0 (or lval == 0). On the
next iteration, scalar arrays increment val by s3->size and jump to stage3,
while string arrays set val = lval + sizeof(char *) and jump to stage2 where
val is now non-zero, bypassing the if (val) check and dereferencing
sizeof(char *) + code->offset.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261112105853.38bd39be@fedora?part=1
^ permalink raw reply [flat|nested] 2+ messages in thread