[PATCH] tracing/probes: Zero out faulted dereferenced fields

[PATCH] tracing/probes: Zero out faulted dereferenced fields

[Steven Rostedt]

From: Steven Rostedt <rostedt@goodmis.org>

While looking at tasks global descriptor tables, I discovered the
following anomaly:

~# echo 't:sched sched_switch prev=prev mm=(task_struct)prev->mm ppgd=(task_struct)prev->mm->pgd' > /sys/kernel/tracing/dynamic_events

The above creates a tprobe on the sched_switch tracepoint and records the
previous task_struct pointer, it's mm_struct and the pgd the mm_struct
points to.

~# trace-cmd start -e tracepoints:sched

            bash-1050    [000] d..3.    90.829478: sched: (__probestub_sched_switch+0x0/0x10) prev=0xffff88d30b623600 mm=0xffff88d30108c600 ppgd=0xffff88d33352b000
          <idle>-0       [002] d..3.    90.836024: sched: (__probestub_sched_switch+0x0/0x10) prev=0xffff88d301870000 mm=0x0 ppgd=0x0
     rcu_preempt-15      [002] d..3.    90.836031: sched: (__probestub_sched_switch+0x0/0x10) prev=0xffff88d3013ad100 mm=0x0 ppgd=0xffff88d33352b000

The first event has is a user task that has both a valid mm_struct and a pgd.
The second event is a kernel task where the mm_struct is NULL and so is the
dereferenced pgd pointer.

The third event is where things looked strange. The mm_struct is NULL but
the dereferenced pgd has a value! That is: NULL->pgd produced something
other than zero.

The reason for this is that the probe_mem_read() and probe_mem_read_user()
do not write to the destination on failure. The process_fetch_insn_bottom()
loop does not check the return value of the dereference (as it is the last
one), and does not update the event. That leaves the value for that failed
field to be whatever was in the ring buffer at that time.

Have both probe_mem_read() and probe_mem_read_user() set the destination to
zero on failure to read the address.

Now all gdb are zero when mm is zero.

Cc: stable@vger.kernel.org
Fixes: 533059281ee59 ("tracing: probeevent: Introduce new argument fetching code")
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_probe_kernel.h | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_probe_kernel.h b/kernel/trace/trace_probe_kernel.h
index 4eb4acfa09ba..00542a162d30 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;
 }
 
 #endif /* __TRACE_PROBE_KERNEL_H_ */
-- 
2.55.0

Re: [PATCH] tracing/probes: Zero out faulted dereferenced fields

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] tracing/probes: uninitialized field on stage 2 fault in
  store_trace_args()
--

commit 721571368356f203544d8ff8abc91509e1c7eea8
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() when a nofault memory read fails so that failed
dereferences do not leave stale ring buffer contents in trace event fields.

> 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: Medium]
Does this still leave the trace event field uninitialized in the ring
buffer when an intermediate dereference faults in stage 2 of
process_fetch_insn_bottom()?

In the commit message's "(task_struct)prev->mm->pgd" example, prev is
valid and prev->mm is NULL, so stage 2 (FETCH_OP_DEREF) succeeds and loads
val = 0, and the fault happens on the final dereference in stage 3
(FETCH_OP_ST_MEM), where dest points to the ring buffer field.

However, if a chain has another dereference after the NULL pointer (such as
"(task_struct)prev->mm->exe_file->f_inode" when prev->mm is NULL), the
fault happens in stage 2:

kernel/trace/trace_probe_tmpl.h:process_fetch_insn_bottom() {
    ...
stage2:
	/* 2nd stage: dereference memory if needed */
	do {
		lval = val;
		switch (code->op) {
		case FETCH_OP_DEREF:
			ret = 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));
			break;
    ...
		}
		if (ret)
			return ret;
    ...
}

Here probe_mem_read() only zeroes the stack variable val, and
process_fetch_insn_bottom() returns early before reaching stage 3.
Because store_trace_args() ignores a negative return value from
process_fetch_insn(), the destination field in the ring buffer is never
written to and retains stale ring buffer data (or, for a dynamic string
argument, keeps the pre-initialized *dl = make_data_loc(maxlen, ...)
header without calling set_data_loc()).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111050322.49bbebfa@fedora?part=1

Re: [PATCH] tracing/probes: Zero out faulted dereferenced fields

[Steven Rostedt]

On Wed, 11 Nov 2026 05:03:22 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> Now all gdb are zero when mm is zero.

That's supposed to be: Now all pgd are zero ...

-- Steve

Re: [PATCH] tracing/probes: Zero out faulted dereferenced fields

[Steven Rostedt]

On Wed, 11 Nov 2026 10:17:13 +0000
sashiko-bot@kernel.org wrote:

> 
> [Severity: Medium]
> Does this still leave the trace event field uninitialized in the ring
> buffer when an intermediate dereference faults in stage 2 of
> process_fetch_insn_bottom()?
> 
> In the commit message's "(task_struct)prev->mm->pgd" example, prev is
> valid and prev->mm is NULL, so stage 2 (FETCH_OP_DEREF) succeeds and loads
> val = 0, and the fault happens on the final dereference in stage 3
> (FETCH_OP_ST_MEM), where dest points to the ring buffer field.
> 
> However, if a chain has another dereference after the NULL pointer (such as
> "(task_struct)prev->mm->exe_file->f_inode" when prev->mm is NULL), the
> fault happens in stage 2:
> 
> kernel/trace/trace_probe_tmpl.h:process_fetch_insn_bottom() {
>     ...
> stage2:
> 	/* 2nd stage: dereference memory if needed */
> 	do {
> 		lval = val;
> 		switch (code->op) {
> 		case FETCH_OP_DEREF:
> 			ret = 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));
> 			break;
>     ...
> 		}
> 		if (ret)
> 			return ret;
>     ...
> }
> 
> Here probe_mem_read() only zeroes the stack variable val, and
> process_fetch_insn_bottom() returns early before reaching stage 3.
> Because store_trace_args() ignores a negative return value from
> process_fetch_insn(), the destination field in the ring buffer is never
> written to and retains stale ring buffer data (or, for a dynamic string
> argument, keeps the pre-initialized *dl = make_data_loc(maxlen, ...)
> header without calling set_data_loc()).
> 

Right.

This is the same issue but in a separate spot. I think this fix should
be folded into this patch. That fix would actually be to ignore the
return value from probe_mem_read*(), as a fault would simply make val
zero, and if we are dereferencing it, we should allow it to continue
with a NULL pointer but detect that too.

-- Steve