Re: [PATCH 09/14] x86/ibt: Implement IBT+
From: Peter Zijlstra
Date: Sat Sep 28 2024 - 09:12:33 EST
On Fri, Sep 27, 2024 at 06:07:33PM -0700, Josh Poimboeuf wrote:
> On Fri, Sep 27, 2024 at 09:49:05PM +0200, Peter Zijlstra wrote:
> > +static void *translate_call_dest(void *dest, bool call)
> > +{
> > + if (cpu_feature_enabled(X86_FEATURE_CALL_DEPTH)) {
> > + if (!call)
> > + return dest;
>
> A tail call is considered a call by virtue of the function name. Change
> the "call" arg to "tail" to make it clearer.
Sure.
> > +++ b/scripts/Makefile.lib
> > @@ -269,6 +269,7 @@ objtool-args-$(CONFIG_HAVE_JUMP_LABEL_HA
> > objtool-args-$(CONFIG_HAVE_NOINSTR_HACK) += --hacks=noinstr
> > objtool-args-$(CONFIG_MITIGATION_CALL_DEPTH_TRACKING) += --direct-call
> > objtool-args-$(CONFIG_X86_KERNEL_IBT) += --ibt
> > +objtool-args-$(CONFIG_X86_KERNEL_IBT_PLUS) += --direct-call
>
> Only add '--direct-call' once:
>
> objtool-args-$(or $(CONFIG_MITIGATION_CALL_DEPTH_TRACKING),$(CONFIG_X86_KERNEL_IBT_PLUS)) += --direct-call
Heh. I don't do enough Makefile to ever remember how they work :/
> > objtool-args-$(CONFIG_FINEIBT) += --cfi
> > objtool-args-$(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL) += --mcount
> > ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
>
> > --- a/tools/objtool/check.c
> > +++ b/tools/objtool/check.c
> > @@ -1455,7 +1455,7 @@ static void annotate_call_site(struct ob
> > return;
> > }
> >
> > - if (!insn->sec->init && !insn->_call_dest->embedded_insn) {
> > + if (!insn->_call_dest->embedded_insn) {
>
> Why did we have the 'init' check to start with? Presumably init memory
> gets freed after the call dest patching so this is not a problem?
Ah, all this came from the calldepth tracking stuff. And we didn't care
about init code, that runs before userspace and so RSB overflows aren't
much of a problem.
But with this here thing, we very much also want to patch the init code
to not land on an ENDBR that will be turned into a UD1 during init :-)