On Thu, 23 Apr 2026 20:38:02 -0700 Josh Poimboeuf <[email protected]> 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 <[email protected]> > 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 <[email protected]> > --- ... > 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
