Re: [PATCH] fprobe: Clear the unused part of the fgraph_data reservation

[Martin Kaiser]

Thus wrote David Carlier (devnexen@xxxxxxxxx):

> fprobe_fgraph_entry() reserves shadow stack space for every fprobe with
> an exit handler, but only fills it for those whose entry handler returns
> 0. fprobe_return() walks the whole reservation, so the unused tail is
> parsed as stale headers from an earlier call, and an exit handler can
> run twice or despite its entry handler asking to skip it.

> The original memset cleared only (reserved_words - used) bytes, and
> commit e0a384434ae1 ("tracing: fprobe: do not zero out unused
> fgraph_data") removed it. Clear the whole tail.

So we're back at

https://lore.kernel.org/all/20260323104818.0ad25dd5@xxxxxxxxxxxxxxxxxx/s

where Steven says

"So fgraph_data is only used internally between the fprobe_fgraph_entry()
and fprobe_return() as it only exists on the fgraph shadow stack. I'm not
even sure if the unused portion needs to be zeroed out."

Looking at this again, it seems to me that your patch makes sense.
AFAICS, fgraph_reserve_data may return memory with dangling data from a
previous call of the traced function.

Best regards,
Martin

> Fixes: 4346ba160409 ("fprobe: Rewrite fprobe on function-graph tracer")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: David Carlier <devnexen@xxxxxxxxx>
> ---
>  kernel/trace/fprobe.c | 4 ++++
>  1 file changed, 4 insertions(+)

> diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
> index 1e9b00997ff2..bd84a982961a 100644
> --- a/kernel/trace/fprobe.c
> +++ b/kernel/trace/fprobe.c
> @@ -635,6 +635,10 @@ static int fprobe_fgraph_entry(struct ftrace_graph_ent *trace, struct fgraph_ops
>  		}
>  	}

> +	/* Clear unused slots so fprobe_return() does not see stale headers. */
> +	if (used < reserved_words)
> +		memset(fgraph_data + used, 0, (reserved_words - used) * sizeof(long));
> +
>  	/* If any exit_handler is set, data must be used. */
>  	return used != 0;
>  }
> -- 
> 2.55.0