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

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

[David Carlier]

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.

Fixes: 4346ba160409 ("fprobe: Rewrite fprobe on function-graph tracer")
Cc: stable@vger.kernel.org
Signed-off-by: David Carlier <devnexen@gmail.com>
---
 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

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

[Martin Kaiser]

Thus wrote David Carlier (devnexen@gmail.com):

> 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@gandalf.local.home/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@vger.kernel.org
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
>  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

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

[Masami Hiramatsu]

On Fri, 11 Sep 2026 20:55:59 +0100
David Carlier <devnexen@gmail.com> wrote:

> 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.

Thanks for reporting! But this does not fix the problem correctly.
See this;

static inline void read_fprobe_header(unsigned long *stack,
					struct fprobe **fp, unsigned int *size_words)
{
	*fp = arch_decode_fprobe_header_fp(*stack);
	*size_words = arch_decode_fprobe_header_size(*stack);
}

#define FPROBE_HEADER_MSB_PATTERN \
    GENMASK(BITS_PER_LONG - 1, FPROBE_HEADER_MSB_SIZE_SHIFT)
#define arch_decode_fprobe_header_fp(val) \
    ((struct fprobe *)(((unsigned long)(val) & FPROBE_HEADER_MSB_MASK) | \
                       FPROBE_HEADER_MSB_PATTERN))

So even if the *stack is zero, the *fp is not NULL.

We need to add *stack check in read_fprobe_handler()s.

Also, since the fprobe_return() exits the loop if fp == NULL,

---
	while (size_words > curr) {
		read_fprobe_header(&fgraph_data[curr], &fp, &size);
		if (!fp)
			break;
---

What we need is writing 0 to stack[used] if used && used < reserved_words
instead of memset.

> 
> Fixes: 4346ba160409 ("fprobe: Rewrite fprobe on function-graph tracer")

And this should be introduced by below commit.

Fixes: e0a384434ae1 ("tracing: fprobe: do not zero out unused fgraph_data")

Thank you,


> Cc: stable@vger.kernel.org
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
>  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
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>