Re: [PATCH] x86/altcall: silence undue warning

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.03.2022 15:23, Andrew Cooper wrote:
> On 01/03/2022 13:34, Jan Beulich wrote:
>> On 01.03.2022 13:48, Andrew Cooper wrote:
>>> On 01/03/2022 11:36, Jan Beulich wrote:
>>>> Suitable compiler options are passed only when the actual feature
>>>> (XEN_IBT) is enabled, not when merely the compiler capability was found
>>>> to be available.
>>>>
>>>> Fixes: 12e3410e071e ("x86/altcall: Check and optimise altcall targets")
>>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> Hmm yes.  This is fallout from separating CONFIG_HAS_CC_CET_IBT and
>>> CONFIG_XEN_IBT.
>>>
>>> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> Thanks.
>>
>>>> ---
>>>> Furthermore, is "Optimised away ..." really appropriate in what
>>>> 37ed5da851b8 ("x86/altcall: Optimise away endbr64 instruction where
>>>> possible") added? If this really was an optimization (rather than
>>>> hardening), shouldn't we purge ENDBR also when !cpu_has_xen_ibt, and
>>>> then ideally all of them? Whereas if this is mainly about hardening,
>>>> wouldn't the message better say "Purged" or "Clobbered"?
>>> I did have an RFC about this.  Turning ENDBR into NOP4 matters, on a
>>> CET-IBT-active system, to restrict the available options an attacker has
>>> when they have already managed to hijack a function pointer (i.e.
>>> already got a partial write gadget).
>>>
>>> From that point of view, it is hardening.
>> But then you don't say whether there's any optimization aspect here.
>> My question was really about the wording of that log message.
> 
> The optimisation aspect is direct branch target +4, because that is what
> saves on decode bandwidth.

But that's the other, entirely independent ENDBR-related piece of code in
the function. All we do in the loop ahead of emitting the message here is
"add_nops(ptr, ENDBR64_LEN)". There's no counting at all of how many
times we advance a call target by 4.

Jan