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

[Steven Rostedt <rostedt@goodmis.org>]

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