| 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
| ||
|
Message-ID: <20260102120913.15e5fc1b@gandalf.local.home>
Date: Fri, 2 Jan 2026 12:09:13 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: oe-kbuild@...ts.linux.dev, "Masami Hiramatsu (Google)"
<mhiramat@...nel.org>, 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 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
?
-- Steve
>
> a1be9ccc57f07d Donglin Peng 2023-04-08 835 #endif
> 7aa1eaef9f4282 Steven Rostedt (VMware 2024-06-03 836)
> 7aa1eaef9f4282 Steven Rostedt (VMware 2024-06-03 837) bitmap = get_bitmap_bits(current, offset);
> 420e1354bcb6f0 Steven Rostedt (Google 2024-06-03 838)
>
Powered by blists - more mailing lists