Re: [PATCH 45/48] x86/Kconfig: Enable CONFIG_PREFIX_SYMBOLS for FineIBT

[David Laight]

On Thu, 23 Apr 2026 20:38:02 -0700
Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:

> On Thu, Apr 23, 2026 at 04:30:47PM -0700, Josh Poimboeuf wrote:
> > On Thu, Apr 23, 2026 at 09:23:12AM -0700, Josh Poimboeuf wrote:  
> > > On Thu, Apr 23, 2026 at 05:19:25PM +0200, Peter Zijlstra wrote:  
> > > > On Thu, Apr 23, 2026 at 08:16:08AM -0700, Josh Poimboeuf wrote:  
> > > > > On Thu, Apr 23, 2026 at 10:47:58AM +0200, Peter Zijlstra wrote:  
> > > > > > On Wed, Apr 22, 2026 at 09:04:13PM -0700, Josh Poimboeuf wrote:  
> > > > > > > PREFIX_SYMBOLS has a !CFI dependency because the compiler already
> > > > > > > generates __cfi_ prefix symbols for kCFI builds, so objtool-generated
> > > > > > > __pfx_ symbols were considered redundant.
> > > > > > > 
> > > > > > > However, the __cfi_ symbols only cover the 5-byte kCFI type hash.  With
> > > > > > > FUNCTION_CALL_PADDING, there are also 11 bytes of NOP padding between
> > > > > > > the hash and the function entry which have no symbol to claim them.  
> > > > > > 
> > > > > > If you force the function alignment to 64 bytes, the prefix will also be
> > > > > > 64bytes, rather than the normal 16.  
> > > > > 
> > > > > Sorry, how do you get 64 here?  
> > > > 
> > > > DEBUG_FORCE_FUNCTION_ALIGNMENT_64B=y  
> > > 
> > > Ok, so in that case it would be 5-byte cfi symbol and 59-byte NOP gap.
> > > Or a 64-byte pfx for the !CFI case.
> > >   
> > > > > > > The NOPs can be rewritten with call depth tracking thunks at runtime.
> > > > > > > Without a symbol, unwinders and other tools that symbolize code
> > > > > > > locations misattribute those bytes.
> > > > > > > 
> > > > > > > Remove the !CFI guard so objtool creates __pfx_ symbols for all
> > > > > > > CALL_PADDING configs, covering the full padding area regardless of
> > > > > > > whether there's also a __cfi_ symbol.  
> > > > > > 
> > > > > > Egads, that a ton of symbols :/ Does it not make sense to 'fix' up the
> > > > > > __cfi_ symbols to cover the whole prefix?  
> > > > > 
> > > > > Yeah, I suppose that would be better, via objtool I presume.  
> > > > 
> > > > Yup.  
> 
> I discovered it's not just FineIBT, it's basically any CALL_PADDING+CFI,
> like so:
> 
> From: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> Subject: [PATCH] objtool: Grow __cfi_* symbols for all kCFI+CALL_PADDING
> 
> For all CONFIG_CFI+CONFIG_CALL_PADDING configs, the __cfi_ symbols only
> cover the 5-byte kCFI type hash.  After that there also N bytes of NOP
> padding between the hash and the function entry which aren't associated
> with any symbol.
> 
> The NOPs can be replaced with actual code at runtime.  Without a symbol,
> unwinders and tooling have no way of knowing where those bytes belong.
> 
> Grow the existing __cfi_* symbols to fill that gap.
> 
> Also, CONFIG_PREFIX_SYMBOLS has no reason to exist: CONFIG_CALL_PADDING
> is what causes the compiler to emit NOP padding before function entry
> (via -fpatchable-function-entry), so it's the right condition for
> creating prefix symbols.
> 
> Remove CONFIG_PREFIX_SYMBOLS, as it's no longer needed.  Simplify the
> LONGEST_SYM_KUNIT_TEST dependency accordingly.
> 
> Update the --cfi and --prefix usage strings to reflect their current
> scope.
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> ---
...
> diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
> index ec7f10a5ef19..254ceb6b0e2c 100644
> --- a/tools/objtool/builtin-check.c
> +++ b/tools/objtool/builtin-check.c
> @@ -73,7 +73,6 @@ static int parse_hacks(const struct option *opt, const char *str, int unset)
>  
>  static const struct option check_options[] = {
>  	OPT_GROUP("Actions:"),
> -	OPT_BOOLEAN(0,		 "cfi", &opts.cfi, "annotate kernel control flow integrity (kCFI) function preambles"),
>  	OPT_STRING_OPTARG('d',	 "disas", &opts.disas, "function-pattern", "disassemble functions", "*"),
>  	OPT_CALLBACK_OPTARG('h', "hacks", NULL, NULL, "jump_label,noinstr,skylake", "patch toolchain bugs/limitations", parse_hacks),
>  	OPT_BOOLEAN('i',	 "ibt", &opts.ibt, "validate and annotate IBT"),
> @@ -84,7 +83,7 @@ static const struct option check_options[] = {
>  	OPT_BOOLEAN('r',	 "retpoline", &opts.retpoline, "validate and annotate retpoline usage"),
>  	OPT_BOOLEAN(0,		 "rethunk", &opts.rethunk, "validate and annotate rethunk usage"),
>  	OPT_BOOLEAN(0,		 "unret", &opts.unret, "validate entry unret placement"),
> -	OPT_INTEGER(0,		 "prefix", &opts.prefix, "generate prefix symbols"),
> +	OPT_INTEGER(0,		 "prefix", &opts.prefix, "generate or grow prefix symbols for N-byte function padding"),
>  	OPT_BOOLEAN('l',	 "sls", &opts.sls, "validate straight-line-speculation mitigations"),
>  	OPT_BOOLEAN('s',	 "stackval", &opts.stackval, "validate frame pointer rules"),
>  	OPT_BOOLEAN('t',	 "static-call", &opts.static_call, "annotate static calls"),
> @@ -92,6 +91,7 @@ static const struct option check_options[] = {
>  	OPT_CALLBACK_OPTARG(0,	 "dump", NULL, NULL, "orc", "dump metadata", parse_dump),
>  
>  	OPT_GROUP("Options:"),
> +	OPT_BOOLEAN(0,		 "cfi", &opts.cfi, "annotate and grow kCFI preamble symbols (use with --prefix)"),
>  	OPT_BOOLEAN(0,		 "backtrace", &opts.backtrace, "unwind on error"),
>  	OPT_BOOLEAN(0,		 "backup", &opts.backup, "create backup (.orig) file on warning/error"),
>  	OPT_BOOLEAN(0,		 "dry-run", &opts.dryrun, "don't write modifications"),
> @@ -163,6 +163,11 @@ static bool opts_valid(void)
>  		return false;
>  	}
>  
> +	if (opts.cfi && !opts.prefix) {
> +		ERROR("--cfi requires --prefix");
> +		return false;
> +	}
> +

Wouldn't it be more friendly to have:
	opts.prefix |= opts.cfi;
and change the help to (implies --prefix).

	David