Re: [PATCH v2 07/70] x86: Build check for embedded endbr64 instructions

[Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>]

On 16/02/2022 08:41, Jan Beulich wrote:
>>> Any zero-padding inserted anywhere by the linker can
>>> result in an immediately following ENDBR to be missed (because
>>> sequences of zeros resemble 2-byte insns).
>> I'm not sure this is a problem.  This pass is looking for everything
>> that objdump thinks is a legal endbr64 instruction, and it splits at labels.
> Oh, right - I did miss the splitting at labels aspect. Hopefully
> objdump is really consistent with this.

Certainly appears to be in my experience.

>>>> +#
>>>> +# Second, look for any endbr64 byte sequence
>>>> +# This has a couple of complications:
>>>> +#
>>>> +# 1) Grep binary search isn't VMA aware.  Copy .text out as binary, 
>>>> causing
>>>> +#    the grep offset to be from the start of .text.
>>>> +#
>>>> +# 2) AWK can't add 64bit integers, because internally all numbers are 
>>>> doubles.
>>>> +#    When the upper bits are set, the exponents worth of precision is 
>>>> lost in
>>>> +#    the lower bits, rounding integers to the nearest 4k.
>>>> +#
>>>> +#    Instead, use the fact that Xen's .text is within a 1G aligned 
>>>> region, and
>>>> +#    split the VMA in half so AWK's numeric addition is only working on 
>>>> 32 bit
>>>> +#    numbers, which don't lose precision.
>>>> +#
>>>> +eval $(${OBJDUMP} -h | awk '$2 == ".text" {printf 
>>>> "vma_hi=%s\nvma_lo=%s\n", substr($4, 1, 8), substr($4, 9, 16)}')
>>>> +
>>>> +${OBJCOPY} -O binary $TEXT_BIN
>>>> +grep -aob "$(printf '\363\17\36\372')" $TEXT_BIN |
>>>> +    awk -F':' '{printf "%s%x\n", "'$vma_hi'", strtonum(0x'$vma_lo') + 
>>>> $1}' > $ALL
>>> None of the three options passed to grep look to be standardized.
>>> Is this going to cause problems on non-Linux systems? Should this
>>> checking perhaps be put behind a separate Kconfig option?
>> CI says that FreeBSD is entirely happy, while Alpine Linux isn't.  This
>> is because Alpine has busybox's grep unless you install the GNU grep
>> package, and I'm doing a fix to our container.
>>
>> My plan to fix this is to just declare a "grep capable of binary
>> searching" a conditional build requirement for Xen.  I don't think this
>> is onerous, and there no other plausible alternatives here.
>>
>> The other option is to detect the absence of support an skip the check. 
>> It is after all a defence in depth scheme, and anything liable to cause
>> a problem would be caught in CI anyway.
> I'd favor the latter approach (but I wouldn't mind the conditional build
> requirement, if you and others deem that better), with a warning issued
> when the check can't be performed. I have to admit that I didn't expect
> there would be no simple and standardized binary search tool on Unix-es.

Ok, so lets do this:

1) This script gets a check for $(grep -aob) and emits a warning to
stderr but exits 0.  This lets people using IBT know that something was
missing.

2) Optional build dependency of `grep -aob` for Xen.  (just a tweak to
README)

3) Update the alpine containers to not miss out.

~Andrew