linux-kernel - Re: [PATCH v4 00/28] objtool: Function validation tracing

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: <zpylbcbpwze5qsyczh43qgg3fjzqh46anvjrixembgosvikub6@ktcdvzwalbyn>
Date: Fri, 14 Nov 2025 13:34:24 -0800
From: Josh Poimboeuf <jpoimboe@...nel.org>
To: Alexandre Chartre <alexandre.chartre@...cle.com>
Cc: linux-kernel@...r.kernel.org, mingo@...nel.org, peterz@...radead.org
Subject: Re: [PATCH v4 00/28] objtool: Function validation tracing

On Fri, Nov 14, 2025 at 10:56:48AM +0100, Alexandre Chartre wrote:
> > Other than '!', the meaning of the flags isn't intuitive.  Maybe it
> > should just show the source code names:
> > 
> >    ALT_NOT(X86_FEATURE_FOO)
> > 
> >    ALT_DIRECT_CALL(X86_FEATURE_BAR)
> > 
> >    ALT_UNKNOWN_FLAG(X86_FEATURE_BAZ)
> > 
> 
> I think '?' is meaningful too, but I wasn't sure about '+'.
> 
> I am using single characters to keep the alternative name short. It can already
> be fairly long because of the feature name (like "X86_FEATURE_SPEC_STORE_BYPASS_DISABLE")
> 
> Also I am assuming that flags can be combined (although that's not currently
> the case) so that would be more difficult with full ALT_* names and the
> result would be much longer.

Ok.

> > > - If an alternative is a jump table then "JUMP" is used as the feature
> > >    name.
> > 
> > Hm, it's a bit confusing to label a jump label as an "alternative" as
> > those are two distinct things (though I'm aware that objtool conflates
> > the two).
> > 
> > > - If an alternative is an exception table then "EXCEPTION" is used as the
> > >    feature name.
> > 
> > Ditto.
> > 
> 
> Yes, the wording is not good, I use it just because objtool handles jump
> labels and exception tables as alternative. I will reword to something
> better.

I meant not only the wording here, but also the "<alternative.def9>"
labels shown in the disassembly.

> > > Disassembly can show default alternative jumping to .altinstr_aux
> > > -----------------------------------------------------------------
> > > Disassembly can show a default alternative jumping to .altinstr_aux. This
> > > happens when the _static_cpu_has() function is used. Its default code
> > > jumps to .altinstr_aux where a test sequence is executed (test; jnz; jmp).
> > > 
> > > At runtime, this sequence is not used because the _static_cpu_has()
> > > an alternative with the X86_FEATURE_ALWAYS feature.
> > > 
> > > 
> > >    debc:  perf_get_x86_pmu_capability+0xc      jmpq   0xdec1 <.altinstr_aux+0xfc> | NOP5  (X86_FEATURE_HYBRID_CPU) | jmpq   0x61a <perf_get_x86_pmu_capability+0x37>  (X86_FEATURE_ALWAYS)   # <alternative.debc>
> > 
> > I'm finding this one-line format considerably more difficult to parse
> > than the slightly longer two-line form:
> > 
> >      debc:  perf_get_x86_pmu_capability+0xc      <alternative.debc>		   | X86_FEATURE_HYBRID_CPU | X86_FEATURE_ALWAYS
> >      debc:  perf_get_x86_pmu_capability+0xc      jmpq   0xdec1 <.altinstr_aux+0xfc> | NOP5		    | jmpq   0x61a <perf_get_x86_pmu_capability+0x37>
> 
> 
> Another option could be:
> 
>     debc:  perf_get_x86_pmu_capability+0xc      jmpq   0xdec1 <.altinstr_aux+0xfc> (<alternative.debc>) |
>                                                 NOP5  (X86_FEATURE_HYBRID_CPU) |
>                                                 jmpq   0x61a <perf_get_x86_pmu_capability+0x37>  (X86_FEATURE_ALWAYS)
> 
> I think I will use this option when displaying alternative one after the other,
> and your suggestion when displaying side-by-side, and add an option to select
> the display.

Hm, but how would that "one after the other" display look for an
alternative with multiple instructions?  A "compact vs wide" option is
ok, but within each of those I think it's helpful to use a consistent
format regardless of whether the alternative has one or multiple
instructions.

> > Also, I wonder if we can make NOP5 lowercase (nop5), since it really is
> > just an instruction, not something special like a feature.
> 
> This indicates that this is a pseudo instruction, NOP5 is actually nopl 0x00(%eax,%eax,1).
> Even NOP1 can be a simple nop but also xchg %rax,%rax.

But it still represents a single instruction... I view it as a readable
shorthand rather than a pseudo instruction.

> > >    82e7:  __switch_to_asm+0x27	  | <alternative.82e7>                   | !X86_FEATURE_ALWAYS                  | X86_FEATURE_RSB_CTXSW
> > 
> > Are the alternatives swapped?  I believe this comes from the following
> > code, so the !X86_FEATURE_ALWAYS column should be last?
> > 
> > .macro FILL_RETURN_BUFFER reg:req nr:req ftr:req ftr2=ALT_NOT(X86_FEATURE_ALWAYS)
> > 	ALTERNATIVE_2 "jmp .Lskip_rsb_\@", \
> > 		__stringify(__FILL_RETURN_BUFFER(\reg,\nr)), \ftr, \
> > 		__stringify(nop;nop;__FILL_ONE_RETURN), \ftr2
> > 
> > .Lskip_rsb_\@:
> > .endm
> 
> I will check but I process/print alternative in the order provided by
> objtool (in struct alternative)

It's quite possible objtool has them in reverse order.  The order
shouldn't matter for objtool validate_branch(), but definitely matters
here.

-- 
Josh