[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 60/70] x86: Build check for embedded endbr64 instructions


  • To: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Wed, 23 Feb 2022 15:29:40 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=4AP0wv/rDw8QLXhERmaitdBAiW7jkpfZhtNATeW13Ko=; b=CA8pzuirXKY1OBtkZnRdCQmthyhkbh4YJbRMbbhdwLsFNmx/DJ9/l8TZqucdsQgFQRINmOebA2t6j/mU6qXlysjs56zVXjem444VUvHA2wPircJPQKb6zlwCq+fyVeSy8ojpE979otVKdUQ7GiIj9A2zxuQYPzHEgrdyxIeaqBtj3QldYxSDMXovoaLFqLsZFMi4cEaodKQdDCrr9Wo66FanoJ770mFUWIzNcyChQmdeRqmiavnWmFf3gR686BOTNrMoYgwHc75s/OjYVCUhfnRfSppDiLuQm4rhGQddXZjiDbaqmUmQCUYhfAoiwo+NylQnq36cm8y+PYJR5ib5VQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=W3DpNYxzkPwZDMqdO57iMMQIGaQULiuUXt/fY5uQ3SofD6Ravt61Q2Dar3NaNFT88ZdkPAlCzojcv2K0FC2OflKl15ZhTvyNnXYGQ4elNlfE3mlOBVLzmAzw0IW2cYRtUuARLmE0zbY1beiYjRrGyr2rHHmW1TsglmBqAk8IDPPbBjiTTj1Bzs6x1QbrN7ysC+tzQpf0tRtJUGbR/HMga6Jtsjh3pkmh+LdpQr2CeKjCrZ53bfK/mPcqNlehD7dIF9mzv0dG5fZEcZpba6SLK2rcMxR79R47zLpC8xPONVW9PlI2aEfH+G0f0KCCjybeLlhwASSZV53Xh473S8vJgw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>, Roger Pau Monne <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 23 Feb 2022 14:29:48 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 23.02.2022 13:05, Andrew Cooper wrote:
> On 23/02/2022 11:31, Jan Beulich wrote:
>> On 22.02.2022 16:26, Andrew Cooper wrote:
>>> up on a non-instruction boundary.  Such embedded instructions mark legal
>>> indirect branch targets as far as the CPU is concerned, which aren't legal 
>>> as
>>> far as the logic is concerned.
>> Thinking about it: Wouldn't it be yet slightly more reassuring to also
>> look for ENDBR32?
> 
> I considered that, but it's awkward to do and doubles the length of this
> already ~0.7s (x2 for efi because this step isn't performed in parallel)
> delay to the build.

(Side note: In general the two linking steps can occur in parallel. An
exception is when the note.o need to be extracted from xen-syms for use
by xen.efi. But that should happen only with old binutils.)

> We do not have __HYPERVISOR_CS32, so ENDBR32 will yield #CP[endbr] if
> encountered.
> 
> If an attacker has managed to edit the GDT to insert a compatibility
> code segment, and hijacked a far transfer to use it, then the absence of
> ENDBR32's in the binary isn't going to be an impediment.

True.

>>> --- /dev/null
>>> +++ b/xen/tools/check-endbr.sh
>>> @@ -0,0 +1,85 @@
>>> +#!/bin/sh
>>> +#
>>> +# Usage ./$0 xen-syms
>>> +#
>>> +set -e
>>> +
>>> +# Prettyprint parameters a little for message
>>> +MSG_PFX="${0##*/} ${1##*/}"
>>> +
>>> +OBJCOPY="${OBJCOPY:-objcopy} -j .text $1"
>>> +OBJDUMP="${OBJDUMP:-objdump} -j .text $1"
>> While embedding the arguments here shortens the lines where these are
>> used, the appearance especially of $OBJCOPY with a single file name
>> argument ...
>>
>>> +ADDR2LINE="${ADDR2LINE:-addr2line}"
>>> +
>>> +D=$(mktemp -d)
>>> +trap "rm -rf $D" EXIT
>>> +
>>> +TEXT_BIN=$D/xen-syms.text
>>> +VALID=$D/valid-addrs
>>> +ALL=$D/all-addrs
>>> +BAD=$D/bad-addrs
>>> +
>>> +# Check that grep can do binary searches.  Some, e.g. busybox, can't.  
>>> Leave a
>>> +# warning but don't fail the build.
>>> +echo "X" | grep -aob "X" -q 2>/dev/null ||
>>> +    { echo "$MSG_PFX Warning: grep can't do binary searches" >&2; exit 0; }
>>> +
>>> +#
>>> +# First, look for all the valid endbr64 instructions.
>>> +# A worst-case disassembly, viewed through cat -A, may look like:
>>> +#
>>> +# ffff82d040337bd4 <endbr64>:$
>>> +# ffff82d040337bd4:^If3 0f 1e fa          ^Iendbr64 $
>>> +# ffff82d040337bd8:^Ieb fe                ^Ijmp    ffff82d040337bd8 
>>> <endbr64+0x4>$
>>> +# ffff82d040337bda:^Ib8 f3 0f 1e fa       ^Imov    $0xfa1e0ff3,%eax$
>>> +#
>>> +# Want to grab the address of endbr64 instructions only, ignoring function
>>> +# names/jump labels/etc, so look for 'endbr64' preceeded by a tab and with 
>>> any
>>> +# number of trailing spaces before the end of the line.
>>> +#
>>> +${OBJDUMP} -d -w | grep '  endbr64 *$' | cut -f 1 -d ':' > $VALID &
>>> +
>>> +#
>>> +# 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) dash's printf doesn't understand hex escapes, hence the use of octal.
>>> +#
>>> +# 3) 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
>> ..., like here, is then somewhat misleading considering that the tool
>> can take one or two filenames as arguments.
> 
> I can re-expand them if you'd prefer.  This would be the delta:

I'd actually be happy to keep "-j .text" where you had it, and merely
move the file arguments to the actual invocation lines. But the way
you have it with the incremental diff is of course even less
"unexpected".

Jan

> diff --git a/xen/tools/check-endbr.sh b/xen/tools/check-endbr.sh
> index 85878353112a..3019ca1c7db0 100755
> --- a/xen/tools/check-endbr.sh
> +++ b/xen/tools/check-endbr.sh
> @@ -7,8 +7,8 @@ set -e
>  # Prettyprint parameters a little for message
>  MSG_PFX="${0##*/} ${1##*/}"
>  
> -OBJCOPY="${OBJCOPY:-objcopy} -j .text $1"
> -OBJDUMP="${OBJDUMP:-objdump} -j .text $1"
> +OBJCOPY="${OBJCOPY:-objcopy}"
> +OBJDUMP="${OBJDUMP:-objdump}"
>  ADDR2LINE="${ADDR2LINE:-addr2line}"
>  
>  D=$(mktemp -d)
> @@ -37,7 +37,7 @@ echo "X" | grep -aob "X" -q 2>/dev/null ||
>  # names/jump labels/etc, so look for 'endbr64' preceeded by a tab and
> with any
>  # number of trailing spaces before the end of the line.
>  #
> -${OBJDUMP} -d -w | grep '      endbr64 *$' | cut -f 1 -d ':' > $VALID &
> +${OBJDUMP} -j .text $1 -d -w | grep '  endbr64 *$' | cut -f 1 -d ':' >
> $VALID &
>  
>  #
>  # Second, look for any endbr64 byte sequence
> @@ -56,9 +56,10 @@ ${OBJDUMP} -d -w | grep '    endbr64 *$' | cut -f 1
> -d ':' > $VALID &
>  #    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)}')
> +eval $(${OBJDUMP} -j .text $1 -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
> +${OBJCOPY} -j .text $1 -O binary $TEXT_BIN
>  grep -aob "$(printf '\363\17\36\372')" $TEXT_BIN |
>      awk -F':' '{printf "%s%x\n", "'$vma_hi'", int(0x'$vma_lo') + $1}' >
> $ALL
>  
> 
> ~Andrew




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.