linux-kernel - Re: [PATCH v2 2/2] objtool: dead_end function change for split functions

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: <CAF1bQ=Tg0+TSM-P+AemJ-EbG1WKf_skyhCD9AxB_VJkc-8dvng@mail.gmail.com>
Date: Mon, 17 Nov 2025 14:57:02 -0800
From: Rong Xu <xur@...gle.com>
To: Josh Poimboeuf <jpoimboe@...nel.org>
Cc: Peter Zijlstra <peterz@...radead.org>, linux-kernel@...r.kernel.org, 
	Sriraman Tallam <tmsriram@...gle.com>, Han Shen <shenhan@...gle.com>, 
	Krzysztof Pszeniczny <kpszeniczny@...gle.com>
Subject: Re: [PATCH v2 2/2] objtool: dead_end function change for split functions

Josh,

You're right, your fix makes a lot of sense. I wasn't aware that the
split function was already handled for GCC.

Yes, clang sets the split part of the function as STT_NOTYPE, which is
different from GCC.

I assume you will post the new patches (together with other fixes for
clang's split function) soon. Any time line?

Also how about the other patch in this patch set. Does that make sense?

Thanks for the advice on getting rid of objtool warnings in our
builds! We tried but it's hard to get rid of all the warnings. But
I'll keep close eyes on these unreachable code warnings.

Thanks,

-Rong

On Mon, Nov 17, 2025 at 1:50 PM Josh Poimboeuf <jpoimboe@...nel.org> wrote:
>
> On Mon, Nov 17, 2025 at 01:43:26PM -0800, Josh Poimboeuf wrote:
> > On Mon, Nov 03, 2025 at 09:52:44PM +0000, xur@...gle.com wrote:
> > > From: Rong Xu <xur@...gle.com>
> > >
> > > Function Splitting can potentially move all return instructions
> > > into the cold (infrequently executed) section of the function.
> > > If this happens, the original function might be incorrectly
> > > flagged as a dead-end function.
> > >
> > > The consequence is an incomplete ORC table, which leads to an unwind
> > > error, and subsequently, a livepatch failure.
> >
> > I assume there were also some ignored objtool warnings?  Ignoring those
> > can have catastrophic consequences (beyond just failing livepatches) so
> > I highly recommend building with CONFIG_OBJTOOL_WERROR.
> >
> > > This patch adds the support of the dead_end_function check for
> > > split function.
> > >
> > > Signed-off-by: Rong Xu <xur@...gle.com>
> > > Reviewed-by: Sriraman Tallam <tmsriram@...gle.com>
> > > Reviewed-by: Han Shen <shenhan@...gle.com>
> > > Reviewed-by: Krzysztof Pszeniczny <kpszeniczny@...gle.com>
> >
> > I was scratching my head about this one for a while.  .text.split.<func>
> > is for <func>.cold code, which should automatically get iterated by the
> > outer func_for_each_insn() construct in __dead_end_function().  So this
> > patch shouldn't be necessary.
> >
> > After some testing with Clang -fsplit-machine-functions, I'm realizing
> > the problem is that unlike GCC, Clang .cold code (in .text.split.*)
> > isn't STT_FUNC.  Which breaks several objtool assumptions.  So something
> > like the below is needed.
>
> That last diff had a blatant bug, here is the proper fix:
>
> diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> index 3f20b257ab25..5a9efbda880b 100644
> --- a/tools/objtool/elf.c
> +++ b/tools/objtool/elf.c
> @@ -502,8 +502,16 @@ static int elf_add_symbol(struct elf *elf, struct symbol *sym)
>         if (strstarts(sym->name, ".klp.sym"))
>                 sym->klp = 1;
>
> -       if (!sym->klp && is_func_sym(sym) && strstr(sym->name, ".cold"))
> +       if (!sym->klp && strstr(sym->name, ".cold")) {
>                 sym->cold = 1;
> +
> +               /*
> +                * Clang doesn't mark cold subfunctions as STT_FUNC, which
> +                * breaks several objtool assumptions.  Fake it.
> +                */
> +               sym->type = STT_FUNC;
> +       }
> +
>         sym->pfunc = sym->cfunc = sym;
>
>         sym->demangled_name = demangle_name(sym);