[PATCH v2] fprobe: Terminate the fgraph_data list when the reservation is not filled

[PATCH v2] fprobe: Terminate the fgraph_data list when the reservation is not filled

[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. fgraph_reserve_data() does not clear the area, so fprobe_return()
parses the unused tail as headers left over from an earlier call, and an
exit handler can run twice or despite its entry handler asking to skip
it.

Write a zero word after the last entry to terminate the walk. A zeroed
slot does not decode to a NULL fprobe on the arches that encode the
header into one unsigned long, since arch_decode_fprobe_header_fp() ORs
in FPROBE_HEADER_MSB_PATTERN, so make read_fprobe_header() return NULL
for a zeroed slot.

Fixes: e0a384434ae1 ("tracing: fprobe: do not zero out unused fgraph_data")
Cc: stable@vger.kernel.org
Suggested-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: David Carlier <devnexen@gmail.com>
---
 kernel/trace/fprobe.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
index 1e9b00997ff2..9f2d98181779 100644
--- a/kernel/trace/fprobe.c
+++ b/kernel/trace/fprobe.c
@@ -171,6 +171,11 @@ static inline bool write_fprobe_header(unsigned long *stack,
 static inline void read_fprobe_header(unsigned long *stack,
 					struct fprobe **fp, unsigned int *size_words)
 {
+	if (!*stack) {
+		*fp = NULL;
+		*size_words = 0;
+		return;
+	}
 	*fp = arch_decode_fprobe_header_fp(*stack);
 	*size_words = arch_decode_fprobe_header_size(*stack);
 }
@@ -203,6 +208,12 @@ static inline void read_fprobe_header(unsigned long *stack,
 {
 	struct __fprobe_header *fph = (struct __fprobe_header *)stack;
 
+	if (!*stack) {
+		*fp = NULL;
+		*size_words = 0;
+		return;
+	}
+
 	*fp = fph->fp;
 	*size_words = fph->size_words;
 }
@@ -635,6 +646,10 @@ static int fprobe_fgraph_entry(struct ftrace_graph_ent *trace, struct fgraph_ops
 		}
 	}
 
+	/* Terminate the list, fgraph_reserve_data() does not clear it. */
+	if (used && used < reserved_words)
+		fgraph_data[used] = 0;
+
 	/* If any exit_handler is set, data must be used. */
 	return used != 0;
 }
-- 
2.55.0

Re: [PATCH v2] fprobe: Terminate the fgraph_data list when the reservation is not filled

[Masami Hiramatsu]

On Thu, 17 Sep 2026 22:24:07 +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. fgraph_reserve_data() does not clear the area, so fprobe_return()
> parses the unused tail as headers left over from an earlier call, and an
> exit handler can run twice or despite its entry handler asking to skip
> it.
> 
> Write a zero word after the last entry to terminate the walk. A zeroed
> slot does not decode to a NULL fprobe on the arches that encode the
> header into one unsigned long, since arch_decode_fprobe_header_fp() ORs
> in FPROBE_HEADER_MSB_PATTERN, so make read_fprobe_header() return NULL
> for a zeroed slot.
> 

This fix looks good to me. Let me pick this.

Thanks!

> Fixes: e0a384434ae1 ("tracing: fprobe: do not zero out unused fgraph_data")
> Cc: stable@vger.kernel.org
> Suggested-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
>  kernel/trace/fprobe.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
> index 1e9b00997ff2..9f2d98181779 100644
> --- a/kernel/trace/fprobe.c
> +++ b/kernel/trace/fprobe.c
> @@ -171,6 +171,11 @@ static inline bool write_fprobe_header(unsigned long *stack,
>  static inline void read_fprobe_header(unsigned long *stack,
>  					struct fprobe **fp, unsigned int *size_words)
>  {
> +	if (!*stack) {
> +		*fp = NULL;
> +		*size_words = 0;
> +		return;
> +	}
>  	*fp = arch_decode_fprobe_header_fp(*stack);
>  	*size_words = arch_decode_fprobe_header_size(*stack);
>  }
> @@ -203,6 +208,12 @@ static inline void read_fprobe_header(unsigned long *stack,
>  {
>  	struct __fprobe_header *fph = (struct __fprobe_header *)stack;
>  
> +	if (!*stack) {
> +		*fp = NULL;
> +		*size_words = 0;
> +		return;
> +	}
> +
>  	*fp = fph->fp;
>  	*size_words = fph->size_words;
>  }
> @@ -635,6 +646,10 @@ static int fprobe_fgraph_entry(struct ftrace_graph_ent *trace, struct fgraph_ops
>  		}
>  	}
>  
> +	/* Terminate the list, fgraph_reserve_data() does not clear it. */
> +	if (used && used < reserved_words)
> +		fgraph_data[used] = 0;
> +
>  	/* If any exit_handler is set, data must be used. */
>  	return used != 0;
>  }
> -- 
> 2.55.0
> 


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