Re: [PATCH] x86/cet: Use dedicated NOP4 for cf_clobber

From: Jan Beulich <jbeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
	"Bjoern Doebel" <doebel@amazon.de>,
	"Michael Kurth" <mku@amazon.de>,
	"Martin Pohlack" <mpohlack@amazon.de>,
	Xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH] x86/cet: Use dedicated NOP4 for cf_clobber
Date: Thu, 17 Mar 2022 11:43:10 +0100	[thread overview]
Message-ID: <db1e9af9-5441-c035-1998-33be07f6e908@suse.com> (raw)
In-Reply-To: <20220317100204.16391-1-andrew.cooper3@citrix.com>

On 17.03.2022 11:02, Andrew Cooper wrote:
> For livepatching, we need to look at a potentially clobbered function and
> determine whether it used to have an ENDBR64 instruction.
> 
> Use a non-default 4-byte P6 long nop, not emitted by toolchains, and extend
> check-endbr.sh to look for it.
> 
> The choice of nop has some complicated consequences.  nopw (%rax) has a ModRM
> byte of 0, which the Bourne compatible shells unconditionally strip from
> parameters, meaning that we can't pass it to `grep -aob`.

Urgh. But as per my earlier comments I'm happier with ...

> Therefore, use nopw (%rcx) so the ModRM byte becomes 1.

... a non-zero ModR/M byte anyway.

> This then demonstrates another bug.  Under perl regexes, \1 thru \9 are
> subpattern matches, and not octal escapes.  Switch the `grep -P` runes to use
> hex escapes instead.
> 
> The build time check then requires that the endbr64 poison have the same
> treatment as endbr64 to avoid placing the byte pattern in immediate operands.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

> Jan: As you had the buggy grep, can you please confirm that hex escapes work.

(Build-)Tested-by: Jan Beulich <jbeulich@suse.com>

When working out the workaround, I actually did test with hex, but
then switched to octal to make easily visible that the two patterns
actually match. I also did wonder about octal and sub-pattern
matching conflicting, but the grep I used definitely didn't have
an issue there. Hence I assume grep behavior changed at some point;
I wonder how they mean to have octal expressed now.
https://perldoc.perl.org/perlre specifically outlines how the
conflict is dealt with - assuming you have observed grep to misbehave,
I wonder whether they've accumulated a bug. (The doc also makes clear
that such references aren't limited to single digit numbers; you may
want to adjust your description in this regard.)

Depending on how exactly your grep behaves, switching to always-three-
digit octal escapes may be an alternative to retain the property of
making obvious the similarity between the two pattern representations.

Jan