Re: [PATCH v3] tracing/probes: bound symstr stores to reserved length

[Masami Hiramatsu (Google)]

On Wed, 04 Nov 2026 17:56:53 +0800, Yifan Zhang <yfzhang23@xxxxxxxxxxxxxx> wrote:
> Traceprobe calculates dynamic string sizes before storing the values.  A
> probed pointer can change between those passes, so
> fetch_store_symstrlen() can reserve space for a short symbol while
> fetch_store_symstring() observes a longer one.  The store path then
> overwrites the trace record.
>
> Format the symbol into a temporary KSYM_SYMBOL_LEN buffer and copy only
> the bytes that fit in the reserved field, including a trailing NUL.
> Update __data_loc with the length actually stored so consumers do not
> read beyond the truncated value.

Good catch! This looks good to me. Let me pick it to probes/fixes.

Thanks!

>
> Fixes: b26a124cbfa8 ("tracing/probes: Add symstr type for dynamic events")
> Signed-off-by: Yifan Zhang <yfzhang23@xxxxxxxxxxxxxx>
> ---
> Changes in v3:
> - encode the actual stored length in __data_loc after truncation
> - encode a zero length if symbol formatting fails
>
>  kernel/trace/trace_probe_tmpl.h | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_probe_tmpl.h b/kernel/trace/trace_probe_tmpl.h
> index 8db12f758fdae..8acc0f0ebd7a6 100644
> --- a/kernel/trace/trace_probe_tmpl.h
> +++ b/kernel/trace/trace_probe_tmpl.h
> @@ -87,15 +87,27 @@ fetch_store_symstrlen(unsigned long addr)
>  static nokprobe_inline int
>  fetch_store_symstring(unsigned long addr, void *dest, void *base)
>  {
> +	char namebuf[KSYM_SYMBOL_LEN];
>  	int maxlen = get_loc_len(*(u32 *)dest);
>  	void *__dest;
> +	int len;
>  
>  	if (unlikely(!maxlen))
>  		return -ENOMEM;
>  
>  	__dest = get_loc_data(dest, base);
> +	len = sprint_symbol(namebuf, addr);
> +	if (len < 0) {
> +		*(u32 *)dest = make_data_loc(0, __dest - base);
> +		return 0;
> +	}
> +
> +	len = min(len, maxlen - 1);
> +	memcpy(__dest, namebuf, len);
> +	((char *)__dest)[len] = '\0';
> +	*(u32 *)dest = make_data_loc(len + 1, __dest - base);
>  
> -	return sprint_symbol(__dest, addr);
> +	return len + 1;
>  }
>  
>  /* common part of process_fetch_insn*/
>

-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>