linux-kernel - Re: [PATCH v6 11/30] objtool: Trace instruction state changes during function validation

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <gl5aug2lwyhjyp2adwjtomxodxixqbpzxj3bcbnimfzo3xmce5@tvpxxghlmxif>
Date: Tue, 2 Dec 2025 08:13:21 -0800
From: Josh Poimboeuf <jpoimboe@...nel.org>
To: Alexandre Chartre <alexandre.chartre@...cle.com>
Cc: Nathan Chancellor <nathan@...nel.org>, linux-kernel@...r.kernel.org, 
	mingo@...nel.org, peterz@...radead.org, david.laight.linux@...il.com, 
	llvm@...ts.linux.dev
Subject: Re: [PATCH v6 11/30] objtool: Trace instruction state changes during
 function validation

On Tue, Dec 02, 2025 at 09:34:20AM +0100, Alexandre Chartre wrote:
> 
> On 12/2/25 02:30, Josh Poimboeuf wrote:
> > On Mon, Dec 01, 2025 at 01:23:29PM -0700, Nathan Chancellor wrote:
> > > Hi Alexandre,
> > > 
> > > On Fri, Nov 21, 2025 at 10:53:21AM +0100, Alexandre Chartre wrote:
> > > > During function validation, objtool maintains a per-instruction state,
> > > > in particular to track call frame information. When tracing validation,
> > > > print any instruction state changes.
> > > > 
> > > > Signed-off-by: Alexandre Chartre <alexandre.chartre@...cle.com>
> > > 
> > > I am seeing a segfault after this change in -next as commit fcb268b47a2f
> > > ("objtool: Trace instruction state changes during function validation")
> > > when building allmodconfig with clang 21.1.6 [1] (I did not check
> > > earlier versions).
> > > 
> > >    $ clang --version | head -1
> > >    ClangBuiltLinux clang version 21.1.6 (https://github.com/llvm/llvm-project.git a832a5222e489298337fbb5876f8dcaf072c5cca)
> > > 
> > >    $ make -skj"$(nproc)" ARCH=x86_64 LLVM=1 clean allmodconfig drivers/scsi/qla2xxx/qla2xxx.o
> > >    make[7]: *** [scripts/Makefile.build:503: drivers/scsi/qla2xxx/qla2xxx.o] Error 139
> > >    ...
> > > 
> > >    $ ld.lld -m elf_x86_64 --fatal-warnings -z noexecstack -r -o drivers/scsi/qla2xxx/qla2xxx.o @drivers/scsi/qla2xxx/qla2xxx.mod
> > > 
> > >    $ tools/objtool/objtool --hacks=jump_label --hacks=noinstr --hacks=skylake --ibt --cfi --mcount --mnop --orc --retpoline --rethunk --sls --static-call --uaccess --no-unreachable --link --module drivers/scsi/qla2xxx/qla2xxx.o
> > >    fish: Job 1, 'tools/objtool/objtool --hacks=j…' terminated by signal SIGSEGV (Address boundary error)
> > > 
> > > If there is any other information I can provide or patches I can test, I
> > > am more than happy to do so.
> > > 
> > > [1]: https://mirrors.edge.kernel.org/pub/tools/llvm/files/llvm-21.1.6-x86_64.tar.xz
> > 
> > Objtool is overflowing the stack due to the large number of jumps it has
> > to follow in that code, thanks to kasan.  The above mentioned patch
> > 
> >    fcb268b47a2f ("objtool: Trace instruction state changes during function validation")
> > 
> > added a 328-byte struct to the stack in validate_insn() which
> > drastically increased the amount of stack size needed.
> > 
> > I suppose we could hack a fix by making it a static local variable, like
> > below.
> > 
> > Or, objtool could setrlimit(RLIMIT_STACK) to 16MB?
> > 
> > diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> > index a02f8db75827..206b8589d82b 100644
> > --- a/tools/objtool/check.c
> > +++ b/tools/objtool/check.c
> > @@ -3678,7 +3678,7 @@ static int validate_insn(struct objtool_file *file, struct symbol *func,
> >   			 bool *dead_end)
> >   {
> >   	/* prev_state is not used if there is no disassembly support */
> > -	struct insn_state prev_state __maybe_unused;
> > +	static struct insn_state prev_state __maybe_unused;
> >   	struct alternative *alt;
> >   	u8 visited;
> >   	int ret;
> 
> static looks good enough to me.
> 
> Reviewed-by: Alexandre Chartre <alexandre.chartre@...cle.com>
> 
> Thanks,

The static local bothered me, I went with a different approach to move
that variable (and its usage) to handle_insn_ops().  Will post shortly.

-- 
Josh