[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 12:31:24 +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=aTT5nBsFvcPRM7gkaZ9hXwUhZEF54Jsjj4N4/hO/hxE=; b=f70gOQRYFeV5uGom1KnaotP78+GV/+s/GwDapB42noznrdfSc3+da+jKAW9FahDAZjKOVxOwN/9qiZMUvNzIZ7iCJL2f/y32z5v7YsVd4EcBCZMCjAu2f+6vv74MjDXyeTX7w0qGcKUR1v05WW0QiyQmS4de8Fgd7pN6ww6lrCHqeM17Q3LofdNVscpMewTAPs+ZrxJkKU51ycU8dy8A9CTXKIc6ghpU9RFrhvD5KWhGel4r1gXHvMbAGrjTESKTE6NfSw6BllLKLXjlszcC8diiszT51AJ6yy4cE7PajiWxyfrOZLttMpbonHsqr1auDCnETI8qSbL9NJjdGyzgrA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=LTK0BQbJ1QP2vfmlmV+Vs7yGu4zhNETzuwXLSAxh6Fau+GjcpISy7T/3hrpjC4UPAi9VQhswm3BDSBV0DQN9LjQtYNdCfVbWvkNBXu6vHKF7IaYx6VQHJXKPWni0LEynsISYXpqM1WqKnP3r+7jXjf4v6OVkEvpO73ijTGJhkySQmV8CJ3H+Nce/OFn1XBiZ33HWBGDi3ur0VXOgZ/EAayd8OuMO/TcMtYu4cfGz5H9iuuSty2xjadD5KXDgu7Olf14mAFi8iyxEDRip/thYBDcYnMlK/Hl1iFfO0ZqfAr5wT4rypctRh1+Avpnk9hrUzPFiWsmXAH+aRcVLR0M0JQ==
  • 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 Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 23 Feb 2022 11:31:38 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 22.02.2022 16:26, Andrew Cooper wrote:
> From: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> 
> An interesting corner case occurs when the byte sequence making up endb64 ends

Nit: For grep-ability it would be nice to spell this "endbr64".

> 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?

> When CET-IBT is active, check for embedded byte sequences.  Example failures
> look like:
> 
>   check-endbr.sh xen-syms Fail: Found 2 embedded endbr64 instructions
>   0xffff82d040325677: test_endbr64 at 
> /local/xen.git/xen/arch/x86/x86_64/entry.S:28
>   0xffff82d040352da6: init_done at /local/xen.git/xen/arch/x86/setup.c:675
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

> --- a/README
> +++ b/README
> @@ -68,6 +68,7 @@ provided by your OS distributor:
>  In addition to the above there are a number of optional build
>  prerequisites. Omitting these will cause the related features to be
>  disabled at compile time:
> +    * Binary-search capable grep (if building Xen with CET support)

Nit: With this (maybe this was the case already earlier though)
s/will/may/ in the previous sentence?

> --- /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.

Jan




 


Rackspace

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