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

[Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>]

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.

>
>> The first version of this logic was trying to use UD1 in the same way as
>> Linux does, to harden non-CET builds too, but that does depend on having
>> objtool so all direct branches can have their targets updated to miss
>> the UD1 instruction.
> I think it would be possible (but likely cumbersome) to arrange for
> this also without objtool.

It's only useful non-CET hardware to cross-check that things won't
explode when CET is enabled, due to mispositioned branches/etc.

An actual attacker on non-CET hardware would just adjust the function
pointer +4 to skip the UD1.

>> P.S. I'd still like to have "away %u of %u endbr64's".  It occurs to me
>> that now we have check-endbr64.sh, we could `wc -l` the $VALID file and
>> re-link this back in, but then we couldn't check the final objects.
> I was thinking about this wish of yours as well; I simply didn't know how
> important you view it to have this information.

The most useful piece of information is how many ENDBR64's remain,
because those denote the extent of the attackers ability to hijack
function pointers without suffering #CP.  Ideally, the number produced
at boot wants cross-checking with script which can take xen-syms,
calculate $VALID - $CF_CLOBBER and identify all the expected
runtime-active targets.

At the moment, I can guestimate based on knowing how many where disabled
at boot time, and how many were in the original build, but I don't
expect anyone else to know that an all-enabled build of Xen is ~1600
ENDBR64's.

~Andrew