2022-04-20 06:25:46

by Andrew Cooper

[permalink] [raw]
Subject: Re: [PATCH v2 06/25] x86/xen: Add ANNOTATE_ENDBR to startup_xen()

On 19/04/2022 12:57, Peter Zijlstra wrote:
> On Tue, Apr 19, 2022 at 11:42:12AM +0000, Andrew Cooper wrote:
>> On 18/04/2022 17:50, Josh Poimboeuf wrote:
>>> The startup_xen() kernel entry point is referenced by the ".note.Xen"
>>> section, but is presumably not indirect-branched to.
>> It's the real entrypoint of the VM.  It's "got to" by setting %rip
>> during vcpu setup.
>>
>> We could in principle support starting a PV VM with CET active, but that
>> sounds like an enormous quantity of effort for very little gain.  CET
>> for Xen PV requires paravirt anyway (because the kernel runs in CPL!=0)
>> so decisions like this can wait until someone feels like doing the work.
>>
>>> Add ANNOTATE_ENDBR
>>> to silence future objtool warnings.
>>>
>>> Cc: Boris Ostrovsky <[email protected]>
>>> Cc: Juergen Gross <[email protected]>
>>> Cc: Stefano Stabellini <[email protected]>
>>> Cc: [email protected]
>>> Signed-off-by: Josh Poimboeuf <[email protected]>
>> FWIW, Reviewed-by: Andrew Cooper <[email protected]>, preferably
>> with the commit message tweaked to remove the uncertainty.
> Something like so then?
>
> ---
> Subject: x86/xen: Add ANNOTATE_ENDBR to startup_xen()
> From: Josh Poimboeuf <[email protected]>
> Date: Mon, 18 Apr 2022 09:50:25 -0700
>
> From: Josh Poimboeuf <[email protected]>
>
> The startup_xen() kernel entry point is referenced by the ".note.Xen"
> section, and is the real entry point of the VM. It *will* be
> indirectly branched to, *however* currently Xen doesn't support PV VM
> with CET active.

Technically it's always IRET'd to, but the point is that it's never
"branched to" by the execution context of the VM.

So it would be better to say that it's never indirectly branched to. 
That's what the IBT checks care about.

>
> Add ANNOTATE_ENDBR to silence future objtool warnings.

Only just spotted.  All text in the subject and commit message needs
s/ENDBR/NOENDBR/

~Andrew


2022-04-22 21:21:10

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH v2 06/25] x86/xen: Add ANNOTATE_ENDBR to startup_xen()

On Tue, Apr 19, 2022 at 01:12:14PM +0100, Andrew Cooper wrote:

> > Subject: x86/xen: Add ANNOTATE_ENDBR to startup_xen()
> > From: Josh Poimboeuf <[email protected]>
> > Date: Mon, 18 Apr 2022 09:50:25 -0700
> >
> > From: Josh Poimboeuf <[email protected]>
> >
> > The startup_xen() kernel entry point is referenced by the ".note.Xen"
> > section, and is the real entry point of the VM. It *will* be
> > indirectly branched to, *however* currently Xen doesn't support PV VM
> > with CET active.
>
> Technically it's always IRET'd to, but the point is that it's never
> "branched to" by the execution context of the VM.
>
> So it would be better to say that it's never indirectly branched to.?
> That's what the IBT checks care about.

Right, so I was thinking the IRET could set the NEED_ENDBR bit, but
yeah, that might be stretching the definition of an indirect-branch a
wee bit.

How about so then?

---
Subject: x86/xen: Add ANNOTATE_NOENDBR to startup_xen()
From: Josh Poimboeuf <[email protected]>
Date: Mon, 18 Apr 2022 09:50:25 -0700

From: Josh Poimboeuf <[email protected]>

The startup_xen() kernel entry point is referenced by the ".note.Xen"
section, and is the real entry point of the VM. Control transfer is
through IRET, which *could* set NEED_ENDBR, however Xen currently does
no such thing.

Add ANNOTATE_NOENDBR to silence future objtool warnings.

Fixes: ed53a0d97192 ("x86/alternative: Use .ibt_endbr_seal to seal indirect calls")
Signed-off-by: Josh Poimboeuf <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Reviewed-by: Andrew Cooper <[email protected]>
Link: https://lkml.kernel.org/r/a87bd48b06d11ec4b98122a429e71e489b4e48c3.1650300597.git.jpoimboe@redhat.com