linux-kernel - Re: kernel/trace/fgraph.c:834 __ftrace_return_to_handler() error: we previously assumed 'fregs' could be null (see line 830)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aVvVrFgh8ieFVbn4@stanley.mountain>
Date: Mon, 5 Jan 2026 18:15:56 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Masami Hiramatsu <mhiramat@...nel.org>
Cc: Steven Rostedt <rostedt@...dmis.org>, oe-kbuild@...ts.linux.dev,
	lkp@...el.com, oe-kbuild-all@...ts.linux.dev,
	linux-kernel@...r.kernel.org
Subject: Re: kernel/trace/fgraph.c:834 __ftrace_return_to_handler() error: we
 previously assumed 'fregs' could be null (see line 830)

On Mon, Jan 05, 2026 at 11:13:55AM +0900, Masami Hiramatsu wrote:
> On Fri, 2 Jan 2026 12:09:13 -0500
> Steven Rostedt <rostedt@...dmis.org> wrote:
> 
> > On Fri, 2 Jan 2026 17:49:39 +0300
> > Dan Carpenter <dan.carpenter@...aro.org> wrote:
> > 
> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > > head:   a919610db43b34621d0c3b333e12db9002caf5da
> > > commit: 2ca8c112c9676e2394d76760db78ffddf21d93b5 fgraph: Pass ftrace_regs to retfunc
> > > config: arm64-randconfig-r073-20251212 (https://download.01.org/0day-ci/archive/20251213/202512131657.JQUt5fXQ-lkp@intel.com/config)
> > > compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 1335a05ab8bc8339ce24be3a9da89d8c3f4e0571)
> > > 
> > > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > > the same patch/commit), kindly add following tags
> > > | Reported-by: kernel test robot <lkp@...el.com>
> > > | Reported-by: Dan Carpenter <dan.carpenter@...aro.org>
> > > | Closes: https://lore.kernel.org/r/202512131657.JQUt5fXQ-lkp@intel.com/
> > > 
> > > smatch warnings:
> > > kernel/trace/fgraph.c:834 __ftrace_return_to_handler() error: we previously assumed 'fregs' could be null (see line 830)
> > > 
> > > vim +/fregs +834 kernel/trace/fgraph.c
> > > 
> > > a3ed4157b7d898 Masami Hiramatsu (Google  2024-12-26  810) static inline unsigned long
> > > a3ed4157b7d898 Masami Hiramatsu (Google  2024-12-26  811) __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointer)
> > > d864a3ca883095 Steven Rostedt (VMware    2018-11-12  812) {
> > > 7aa1eaef9f4282 Steven Rostedt (VMware    2024-06-03  813) 	struct ftrace_ret_stack *ret_stack;
> > > d864a3ca883095 Steven Rostedt (VMware    2018-11-12  814) 	struct ftrace_graph_ret trace;
> > > 7aa1eaef9f4282 Steven Rostedt (VMware    2024-06-03  815) 	unsigned long bitmap;
> > > d864a3ca883095 Steven Rostedt (VMware    2018-11-12  816) 	unsigned long ret;
> > > 7aa1eaef9f4282 Steven Rostedt (VMware    2024-06-03  817) 	int offset;
> > > 7aa1eaef9f4282 Steven Rostedt (VMware    2024-06-03  818) 	int i;
> > > 7aa1eaef9f4282 Steven Rostedt (VMware    2024-06-03  819) 
> > > 7aa1eaef9f4282 Steven Rostedt (VMware    2024-06-03  820) 	ret_stack = ftrace_pop_return_trace(&trace, &ret, frame_pointer, &offset);
> > > 7aa1eaef9f4282 Steven Rostedt (VMware    2024-06-03  821) 
> > > 7aa1eaef9f4282 Steven Rostedt (VMware    2024-06-03  822) 	if (unlikely(!ret_stack)) {
> > > 7aa1eaef9f4282 Steven Rostedt (VMware    2024-06-03  823) 		ftrace_graph_stop();
> > > 7aa1eaef9f4282 Steven Rostedt (VMware    2024-06-03  824) 		WARN_ON(1);
> > > 7aa1eaef9f4282 Steven Rostedt (VMware    2024-06-03  825) 		/* Might as well panic. What else to do? */
> > > 7aa1eaef9f4282 Steven Rostedt (VMware    2024-06-03  826) 		return (unsigned long)panic;
> > > 7aa1eaef9f4282 Steven Rostedt (VMware    2024-06-03  827) 	}
> > > d864a3ca883095 Steven Rostedt (VMware    2018-11-12  828) 
> > > 7aa1eaef9f4282 Steven Rostedt (VMware    2024-06-03  829) 	trace.rettime = trace_clock_local();
> > > 2ca8c112c9676e Masami Hiramatsu (Google  2024-12-26 @830) 	if (fregs)
> > > 
> > > It's strange that Smatch is only now complaining about something which
> > > is from 2024...  This line assumes "freqs" can be NULL.
> > 
> > Hmm, OK, I can see why smatch is complaining. But it is missing a dependency.
> > 
> > This is called by:
> > 
> > #ifdef CONFIG_HAVE_FUNCTION_GRAPH_FREGS
> > unsigned long ftrace_return_to_handler(struct ftrace_regs *fregs)
> > {
> > 	return __ftrace_return_to_handler(fregs,
> > 				ftrace_regs_get_frame_pointer(fregs));
> > }
> > #else
> > unsigned long ftrace_return_to_handler(unsigned long frame_pointer)
> > {
> > 	return __ftrace_return_to_handler(NULL, frame_pointer);
> > }
> > #endif
> > 
> > Where if HAVE_FUNCTION_GRAPH_FREGS is true, then fregs is always set.
> > 
> > > 
> > > 2ca8c112c9676e Masami Hiramatsu (Google  2024-12-26  831) 		ftrace_regs_set_instruction_pointer(fregs, ret);
> > > 2ca8c112c9676e Masami Hiramatsu (Google  2024-12-26  832) 
> > > a1be9ccc57f07d Donglin Peng              2023-04-08  833  #ifdef CONFIG_FUNCTION_GRAPH_RETVAL
> > > a3ed4157b7d898 Masami Hiramatsu (Google  2024-12-26 @834) 	trace.retval = ftrace_regs_get_return_value(fregs);
> > >                                                                                                             ^^^^^
> > > Unchecked dereference.
> > 
> > Here FUNCTION_GRAPH_RETVAL has this in the Kconfig:
> > 
> > config FUNCTION_GRAPH_RETVAL
> > 	bool "Kernel Function Graph Return Value"
> > 	depends on HAVE_FUNCTION_GRAPH_FREGS
> > 
> > It depends on HAVE_FUNCTION_GRAPH_FREGS
> > 
> > Thus, it can't be called without fregs being valid.
> > 
> > Ideas on how to tell smatch about this?
> > 
> > Hmm, I wonder if we could change this to:
> > 
> > 	/* fregs is always set when configured */
> > 	if (IS_ENABLED(CONFIG_HAVE_FUNCTION_GRAPH_FREGS))
> > 		ftrace_regs_set_instruction_pointer(fregs, ret);
> > 
> > And also add:
> > 
> > #ifdef CONFIG_FUNCTION_GRAPH_RETVAL
> > 	/* The below is always true, but makes smatch happy */
> > 	if (IS_ENABLED(CONFIG_HAVE_FUNCTION_GRAPH_FREGS))
> > 		trace.retval = ftrace_regs_get_return_value(fregs);
> > #endif
> > 
> > ?
> 
> Can smatch understand fregs is not NULL if the
> CONFIG_HAVE_FUNCTION_GRAPH_FREGS=y? If it can, I think this
> can skip unneeded NULL pointer check at runtime.
> 
> (Or, define those macros to do nothing if CONFIG_HAVE_FUNCTION_GRAPH_FREGS=n)
> 
> Thank you,

We could just ignore this...  The explanation is on the list if people
have questions.

The problem is that this is only called from assembly which smatch
doesn't understand.

regards,
dan carpenter