* [PATCH v5 1/2] Livepatch: resolve old address before function verification
@ 2022-03-09 14:52 Bjoern Doebel
2022-03-09 14:53 ` [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-enhanced functions Bjoern Doebel
0 siblings, 1 reply; 8+ messages in thread
From: Bjoern Doebel @ 2022-03-09 14:52 UTC (permalink / raw)
To: xen-devel
Cc: Michael Kurth, Martin Pohlack, Roger Pau Monne, Andrew Cooper,
Bjoern Doebel, Konrad Rzeszutek Wilk, Ross Lagerwall
When verifying that a livepatch can be applied, we may as well want to
inspect the target function to be patched. To do so, we need to resolve
this function's address before running the arch-specific
livepatch_verify hook.
Signed-off-by: Bjoern Doebel <doebel@amazon.de>
Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
xen/common/livepatch.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index ec301a9f12..be2cf75c2d 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -684,11 +684,11 @@ static int prepare_payload(struct payload *payload,
return -EINVAL;
}
- rc = arch_livepatch_verify_func(f);
+ rc = resolve_old_address(f, elf);
if ( rc )
return rc;
- rc = resolve_old_address(f, elf);
+ rc = arch_livepatch_verify_func(f);
if ( rc )
return rc;
--
2.32.0
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-enhanced functions
2022-03-09 14:52 [PATCH v5 1/2] Livepatch: resolve old address before function verification Bjoern Doebel
@ 2022-03-09 14:53 ` Bjoern Doebel
2022-03-09 15:14 ` Jan Beulich
2022-03-09 17:12 ` Ross Lagerwall
0 siblings, 2 replies; 8+ messages in thread
From: Bjoern Doebel @ 2022-03-09 14:53 UTC (permalink / raw)
To: xen-devel
Cc: Michael Kurth, Martin Pohlack, Roger Pau Monne, Andrew Cooper,
Bjoern Doebel, Konrad Rzeszutek Wilk, Ross Lagerwall
Xen enabled CET for supporting architectures. The control flow aspect of
CET expects functions that can be called indirectly (i.e., via function
pointers) to start with an ENDBR64 instruction. Otherwise a control flow
exception is raised.
This expectation breaks livepatching flows because we patch functions by
overwriting their first 5 bytes with a JMP + <offset>, thus breaking the
ENDBR64. We fix this by checking the start of a patched function for
being ENDBR64. In the positive case we move the livepatch JMP to start
behind the ENDBR64 instruction.
To avoid having to guess the ENDBR64 offset again on patch reversal
(which might race with other mechanisms adding/removing ENDBR
dynamically), use the livepatch metadata to store the computed offset
along with the saved bytes of the overwritten function.
Signed-off-by: Bjoern Doebel <doebel@amazon.de>
Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Ross Lagerwall <ross.lagerwall@citrix.com>
----
Changes since r1:
* use sizeof_field() to avoid unused variable warning
* make metadata variable const in arch_livepatch_revert
* rebase on top and make use of Andrew Cooper's was_endbr64() patch
* use padding byte to store offset rather than reducing opaque area
---
xen/arch/x86/livepatch.c | 39 +++++++++++++++++++++++++++++++------
xen/include/public/sysctl.h | 3 ++-
2 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index 37c9b8435e..e71741743a 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -14,6 +14,7 @@
#include <xen/vm_event.h>
#include <xen/virtual_region.h>
+#include <asm/endbr.h>
#include <asm/fixmap.h>
#include <asm/nmi.h>
#include <asm/livepatch.h>
@@ -114,8 +115,21 @@ int arch_livepatch_verify_func(const struct livepatch_func *func)
if ( func->old_size < func->new_size )
return -EINVAL;
}
- else if ( func->old_size < ARCH_PATCH_INSN_SIZE )
- return -EINVAL;
+ else
+ {
+ /*
+ * Space needed now depends on whether the target function
+ * start{s,ed} with an ENDBR64 instruction.
+ */
+ uint8_t needed;
+
+ needed = ARCH_PATCH_INSN_SIZE;
+ if ( is_endbr64(func->old_addr) || was_endbr64(func->old_addr) )
+ needed += ENDBR64_LEN;
+
+ if ( func->old_size < needed )
+ return -EINVAL;
+ }
return 0;
}
@@ -130,12 +144,24 @@ void noinline arch_livepatch_apply(struct livepatch_func *func)
uint8_t insn[sizeof(func->opaque)];
unsigned int len;
+ func->patch_offset = 0;
old_ptr = func->old_addr;
len = livepatch_insn_len(func);
if ( !len )
return;
- memcpy(func->opaque, old_ptr, len);
+ /*
+ * CET hotpatching support: We may have functions starting with an ENDBR64
+ * instruction that MUST remain the first instruction of the function, hence
+ * we need to move any hotpatch trampoline further into the function. For that
+ * we need to keep track of the patching offset used for any loaded hotpatch
+ * (to avoid racing against other fixups adding/removing ENDBR64 or similar
+ * instructions).
+ */
+ if ( is_endbr64(old_ptr) || was_endbr64(func->old_addr) )
+ func->patch_offset += ENDBR64_LEN;
+
+ memcpy(func->opaque, old_ptr + func->patch_offset, len);
if ( func->new_addr )
{
int32_t val;
@@ -143,14 +169,15 @@ void noinline arch_livepatch_apply(struct livepatch_func *func)
BUILD_BUG_ON(ARCH_PATCH_INSN_SIZE != (1 + sizeof(val)));
insn[0] = 0xe9; /* Relative jump. */
- val = func->new_addr - func->old_addr - ARCH_PATCH_INSN_SIZE;
+ val = func->new_addr - (func->old_addr + func->patch_offset
+ + ARCH_PATCH_INSN_SIZE);
memcpy(&insn[1], &val, sizeof(val));
}
else
add_nops(insn, len);
- memcpy(old_ptr, insn, len);
+ memcpy(old_ptr + func->patch_offset, insn, len);
}
/*
@@ -159,7 +186,7 @@ void noinline arch_livepatch_apply(struct livepatch_func *func)
*/
void noinline arch_livepatch_revert(const struct livepatch_func *func)
{
- memcpy(func->old_addr, func->opaque, livepatch_insn_len(func));
+ memcpy(func->old_addr + func->patch_offset, func->opaque, livepatch_insn_len(func));
}
/*
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 55252e97f2..b0a4af8789 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -876,7 +876,8 @@ struct livepatch_func {
uint8_t version; /* MUST be LIVEPATCH_PAYLOAD_VERSION. */
uint8_t opaque[LIVEPATCH_OPAQUE_SIZE];
uint8_t applied;
- uint8_t _pad[7];
+ uint8_t patch_offset;
+ uint8_t _pad[6];
livepatch_expectation_t expect;
};
typedef struct livepatch_func livepatch_func_t;
--
2.32.0
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-enhanced functions
2022-03-09 14:53 ` [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-enhanced functions Bjoern Doebel
@ 2022-03-09 15:14 ` Jan Beulich
2022-03-09 17:12 ` Ross Lagerwall
1 sibling, 0 replies; 8+ messages in thread
From: Jan Beulich @ 2022-03-09 15:14 UTC (permalink / raw)
To: Bjoern Doebel
Cc: Michael Kurth, Martin Pohlack, Roger Pau Monne, Andrew Cooper,
Konrad Rzeszutek Wilk, Ross Lagerwall, xen-devel
On 09.03.2022 15:53, Bjoern Doebel wrote:
> Changes since r1:
> * use sizeof_field() to avoid unused variable warning
> * make metadata variable const in arch_livepatch_revert
> * rebase on top and make use of Andrew Cooper's was_endbr64() patch
> * use padding byte to store offset rather than reducing opaque area
You seem to accumulate things here, thus making it impossible to spot
what was changed from the previous version. Retaining all changes
information is helpful, but it wants splitting up suitably.
It would also have been helpful if you had mentioned the 2nd
was_endbr64() that has appeared now.
I'm glad to see the casts are gone now, thanks.
> @@ -114,8 +115,21 @@ int arch_livepatch_verify_func(const struct livepatch_func *func)
> if ( func->old_size < func->new_size )
> return -EINVAL;
> }
> - else if ( func->old_size < ARCH_PATCH_INSN_SIZE )
> - return -EINVAL;
> + else
> + {
> + /*
> + * Space needed now depends on whether the target function
> + * start{s,ed} with an ENDBR64 instruction.
> + */
> + uint8_t needed;
> +
> + needed = ARCH_PATCH_INSN_SIZE;
Surely this can be the initializer of the variable?
> @@ -130,12 +144,24 @@ void noinline arch_livepatch_apply(struct livepatch_func *func)
> uint8_t insn[sizeof(func->opaque)];
> unsigned int len;
>
> + func->patch_offset = 0;
> old_ptr = func->old_addr;
> len = livepatch_insn_len(func);
> if ( !len )
> return;
>
> - memcpy(func->opaque, old_ptr, len);
> + /*
> + * CET hotpatching support: We may have functions starting with an ENDBR64
> + * instruction that MUST remain the first instruction of the function, hence
> + * we need to move any hotpatch trampoline further into the function. For that
> + * we need to keep track of the patching offset used for any loaded hotpatch
> + * (to avoid racing against other fixups adding/removing ENDBR64 or similar
> + * instructions).
> + */
Bad indentation of all but the first line of this comment. Also the
middle on of the lines is too long.
> + if ( is_endbr64(old_ptr) || was_endbr64(func->old_addr) )
Stray double blank in the middle.
> @@ -143,14 +169,15 @@ void noinline arch_livepatch_apply(struct livepatch_func *func)
> BUILD_BUG_ON(ARCH_PATCH_INSN_SIZE != (1 + sizeof(val)));
>
> insn[0] = 0xe9; /* Relative jump. */
> - val = func->new_addr - func->old_addr - ARCH_PATCH_INSN_SIZE;
> + val = func->new_addr - (func->old_addr + func->patch_offset
> + + ARCH_PATCH_INSN_SIZE);
Nit: On split lines the operator still goes on the previous line. (We
often make an exception for ?:, but not normally anything else.)
> @@ -159,7 +186,7 @@ void noinline arch_livepatch_apply(struct livepatch_func *func)
> */
> void noinline arch_livepatch_revert(const struct livepatch_func *func)
> {
> - memcpy(func->old_addr, func->opaque, livepatch_insn_len(func));
> + memcpy(func->old_addr + func->patch_offset, func->opaque, livepatch_insn_len(func));
This line is now too long.
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-enhanced functions
2022-03-09 14:53 ` [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-enhanced functions Bjoern Doebel
2022-03-09 15:14 ` Jan Beulich
@ 2022-03-09 17:12 ` Ross Lagerwall
2022-03-17 9:17 ` Jiamei Xie
1 sibling, 1 reply; 8+ messages in thread
From: Ross Lagerwall @ 2022-03-09 17:12 UTC (permalink / raw)
To: Bjoern Doebel, xen-devel
Cc: Michael Kurth, Martin Pohlack, Roger Pau Monne, Andrew Cooper,
Konrad Rzeszutek Wilk
> From: Bjoern Doebel <doebel@amazon.de>
> Sent: Wednesday, March 9, 2022 2:53 PM
> To: xen-devel@lists.xenproject.org <xen-devel@lists.xenproject.org>
> Cc: Michael Kurth <mku@amazon.de>; Martin Pohlack <mpohlack@amazon.de>; Roger Pau Monne <roger.pau@citrix.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>; Bjoern Doebel <doebel@amazon.de>; Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; Ross Lagerwall <ross.lagerwall@citrix.com>
> Subject: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-enhanced functions
>
> Xen enabled CET for supporting architectures. The control flow aspect of
> CET expects functions that can be called indirectly (i.e., via function
> pointers) to start with an ENDBR64 instruction. Otherwise a control flow
> exception is raised.
>
> This expectation breaks livepatching flows because we patch functions by
> overwriting their first 5 bytes with a JMP + <offset>, thus breaking the
> ENDBR64. We fix this by checking the start of a patched function for
> being ENDBR64. In the positive case we move the livepatch JMP to start
> behind the ENDBR64 instruction.
>
> To avoid having to guess the ENDBR64 offset again on patch reversal
> (which might race with other mechanisms adding/removing ENDBR
> dynamically), use the livepatch metadata to store the computed offset
> along with the saved bytes of the overwritten function.
>
> Signed-off-by: Bjoern Doebel <doebel@amazon.de>
> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> CC: Ross Lagerwall <ross.lagerwall@citrix.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-enhanced functions
2022-03-09 17:12 ` Ross Lagerwall
@ 2022-03-17 9:17 ` Jiamei Xie
2022-03-17 10:00 ` Jiamei Xie
0 siblings, 1 reply; 8+ messages in thread
From: Jiamei Xie @ 2022-03-17 9:17 UTC (permalink / raw)
To: Ross Lagerwall, Bjoern Doebel, xen-devel
Cc: Michael Kurth, Martin Pohlack, Roger Pau Monne, Andrew Cooper,
Konrad Rzeszutek Wilk
Hi Bjoern,
> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of
> Ross Lagerwall
> Sent: 2022年3月10日 1:12
> To: Bjoern Doebel <doebel@amazon.de>; xen-devel@lists.xenproject.org
> Cc: Michael Kurth <mku@amazon.de>; Martin Pohlack
> <mpohlack@amazon.de>; Roger Pau Monne <roger.pau@citrix.com>;
> Andrew Cooper <Andrew.Cooper3@citrix.com>; Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com>
> Subject: Re: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-
> enhanced functions
>
> > From: Bjoern Doebel <doebel@amazon.de>
> > Sent: Wednesday, March 9, 2022 2:53 PM
> > To: xen-devel@lists.xenproject.org <xen-devel@lists.xenproject.org>
> > Cc: Michael Kurth <mku@amazon.de>; Martin Pohlack
> <mpohlack@amazon.de>; Roger Pau Monne <roger.pau@citrix.com>;
> Andrew Cooper <Andrew.Cooper3@citrix.com>; Bjoern Doebel
> <doebel@amazon.de>; Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>;
> Ross Lagerwall <ross.lagerwall@citrix.com>
> > Subject: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-
> enhanced functions
> >
> > Xen enabled CET for supporting architectures. The control flow aspect of
> > CET expects functions that can be called indirectly (i.e., via function
> > pointers) to start with an ENDBR64 instruction. Otherwise a control flow
> > exception is raised.
> >
> > This expectation breaks livepatching flows because we patch functions by
> > overwriting their first 5 bytes with a JMP + <offset>, thus breaking the
> > ENDBR64. We fix this by checking the start of a patched function for
> > being ENDBR64. In the positive case we move the livepatch JMP to start
> > behind the ENDBR64 instruction.
> >
> > To avoid having to guess the ENDBR64 offset again on patch reversal
> > (which might race with other mechanisms adding/removing ENDBR
> > dynamically), use the livepatch metadata to store the computed offset
> > along with the saved bytes of the overwritten function.
> >
> > Signed-off-by: Bjoern Doebel <doebel@amazon.de>
> > Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > CC: Ross Lagerwall <ross.lagerwall@citrix.com>
>
> Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Tested-by: Jiamei xie <jiamei.xie@arm.com>
Cheers,
Jiamei
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-enhanced functions
2022-03-17 9:17 ` Jiamei Xie
@ 2022-03-17 10:00 ` Jiamei Xie
2022-03-17 10:11 ` Jan Beulich
2022-03-17 13:10 ` Doebel, Bjoern
0 siblings, 2 replies; 8+ messages in thread
From: Jiamei Xie @ 2022-03-17 10:00 UTC (permalink / raw)
To: Jiamei Xie, Ross Lagerwall, Bjoern Doebel, xen-devel
Cc: Michael Kurth, Martin Pohlack, Roger Pau Monne, Andrew Cooper,
Konrad Rzeszutek Wilk
> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of
> Jiamei Xie
> Sent: 2022年3月17日 17:17
> To: Ross Lagerwall <ross.lagerwall@citrix.com>; Bjoern Doebel
> <doebel@amazon.de>; xen-devel@lists.xenproject.org
> Cc: Michael Kurth <mku@amazon.de>; Martin Pohlack
> <mpohlack@amazon.de>; Roger Pau Monne <roger.pau@citrix.com>;
> Andrew Cooper <Andrew.Cooper3@citrix.com>; Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com>
> Subject: RE: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-
> enhanced functions
>
> Hi Bjoern,
>
> > -----Original Message-----
> > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of
> > Ross Lagerwall
> > Sent: 2022年3月10日 1:12
> > To: Bjoern Doebel <doebel@amazon.de>; xen-devel@lists.xenproject.org
> > Cc: Michael Kurth <mku@amazon.de>; Martin Pohlack
> > <mpohlack@amazon.de>; Roger Pau Monne <roger.pau@citrix.com>;
> > Andrew Cooper <Andrew.Cooper3@citrix.com>; Konrad Rzeszutek Wilk
> > <konrad.wilk@oracle.com>
> > Subject: Re: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-
> > enhanced functions
> >
> > > From: Bjoern Doebel <doebel@amazon.de>
> > > Sent: Wednesday, March 9, 2022 2:53 PM
> > > To: xen-devel@lists.xenproject.org <xen-devel@lists.xenproject.org>
> > > Cc: Michael Kurth <mku@amazon.de>; Martin Pohlack
> > <mpohlack@amazon.de>; Roger Pau Monne <roger.pau@citrix.com>;
> > Andrew Cooper <Andrew.Cooper3@citrix.com>; Bjoern Doebel
> > <doebel@amazon.de>; Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>;
> > Ross Lagerwall <ross.lagerwall@citrix.com>
> > > Subject: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-
> > enhanced functions
> > >
> > > Xen enabled CET for supporting architectures. The control flow aspect of
> > > CET expects functions that can be called indirectly (i.e., via function
> > > pointers) to start with an ENDBR64 instruction. Otherwise a control flow
> > > exception is raised.
> > >
> > > This expectation breaks livepatching flows because we patch functions by
> > > overwriting their first 5 bytes with a JMP + <offset>, thus breaking the
> > > ENDBR64. We fix this by checking the start of a patched function for
> > > being ENDBR64. In the positive case we move the livepatch JMP to start
> > > behind the ENDBR64 instruction.
> > >
> > > To avoid having to guess the ENDBR64 offset again on patch reversal
> > > (which might race with other mechanisms adding/removing ENDBR
> > > dynamically), use the livepatch metadata to store the computed offset
> > > along with the saved bytes of the overwritten function.
> > >
> > > Signed-off-by: Bjoern Doebel <doebel@amazon.de>
> > > Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > > CC: Ross Lagerwall <ross.lagerwall@citrix.com>
> >
> > Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
>
> Tested-by: Jiamei xie <jiamei.xie@arm.com>
>
> Cheers,
> Jiamei
Sorry I forgot to add the scope I tested in last email. I tested it on armv8a. It worked fine and didn't break arm.
Tested-by: Jiamei xie <jiamei.xie@arm.com>
> Cheers,
> Jiamei
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-enhanced functions
2022-03-17 10:00 ` Jiamei Xie
@ 2022-03-17 10:11 ` Jan Beulich
2022-03-17 13:10 ` Doebel, Bjoern
1 sibling, 0 replies; 8+ messages in thread
From: Jan Beulich @ 2022-03-17 10:11 UTC (permalink / raw)
To: Jiamei Xie
Cc: Michael Kurth, Martin Pohlack, Roger Pau Monne, Andrew Cooper,
Konrad Rzeszutek Wilk, Ross Lagerwall, Bjoern Doebel, xen-devel
On 17.03.2022 11:00, Jiamei Xie wrote:
>> -----Original Message-----
>> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of
>> Jiamei Xie
>> Sent: 2022年3月17日 17:17
>>
>>> -----Original Message-----
>>> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of
>>> Ross Lagerwall
>>> Sent: 2022年3月10日 1:12
>>> To: Bjoern Doebel <doebel@amazon.de>; xen-devel@lists.xenproject.org
>>> Cc: Michael Kurth <mku@amazon.de>; Martin Pohlack
>>> <mpohlack@amazon.de>; Roger Pau Monne <roger.pau@citrix.com>;
>>> Andrew Cooper <Andrew.Cooper3@citrix.com>; Konrad Rzeszutek Wilk
>>> <konrad.wilk@oracle.com>
>>> Subject: Re: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-
>>> enhanced functions
>>>
>>>> From: Bjoern Doebel <doebel@amazon.de>
>>>> Sent: Wednesday, March 9, 2022 2:53 PM
>>>> To: xen-devel@lists.xenproject.org <xen-devel@lists.xenproject.org>
>>>> Cc: Michael Kurth <mku@amazon.de>; Martin Pohlack
>>> <mpohlack@amazon.de>; Roger Pau Monne <roger.pau@citrix.com>;
>>> Andrew Cooper <Andrew.Cooper3@citrix.com>; Bjoern Doebel
>>> <doebel@amazon.de>; Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>;
>>> Ross Lagerwall <ross.lagerwall@citrix.com>
>>>> Subject: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-
>>> enhanced functions
>>>>
>>>> Xen enabled CET for supporting architectures. The control flow aspect of
>>>> CET expects functions that can be called indirectly (i.e., via function
>>>> pointers) to start with an ENDBR64 instruction. Otherwise a control flow
>>>> exception is raised.
>>>>
>>>> This expectation breaks livepatching flows because we patch functions by
>>>> overwriting their first 5 bytes with a JMP + <offset>, thus breaking the
>>>> ENDBR64. We fix this by checking the start of a patched function for
>>>> being ENDBR64. In the positive case we move the livepatch JMP to start
>>>> behind the ENDBR64 instruction.
>>>>
>>>> To avoid having to guess the ENDBR64 offset again on patch reversal
>>>> (which might race with other mechanisms adding/removing ENDBR
>>>> dynamically), use the livepatch metadata to store the computed offset
>>>> along with the saved bytes of the overwritten function.
>>>>
>>>> Signed-off-by: Bjoern Doebel <doebel@amazon.de>
>>>> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>>> CC: Ross Lagerwall <ross.lagerwall@citrix.com>
>>>
>>> Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
>>
>> Tested-by: Jiamei xie <jiamei.xie@arm.com>
>>
>> Cheers,
>> Jiamei
> Sorry I forgot to add the scope I tested in last email. I tested it on armv8a. It worked fine and didn't break arm.
> Tested-by: Jiamei xie <jiamei.xie@arm.com>
Yet in any event there's meanwhile been a v6, so I'm unsure of taking the
tag over there.
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-enhanced functions
2022-03-17 10:00 ` Jiamei Xie
2022-03-17 10:11 ` Jan Beulich
@ 2022-03-17 13:10 ` Doebel, Bjoern
1 sibling, 0 replies; 8+ messages in thread
From: Doebel, Bjoern @ 2022-03-17 13:10 UTC (permalink / raw)
To: Jiamei Xie, Ross Lagerwall, xen-devel
Cc: Michael Kurth, Martin Pohlack, Roger Pau Monne, Andrew Cooper,
Konrad Rzeszutek Wilk
On 17.03.22 11:00, Jiamei Xie wrote:
>
>> -----Original Message-----
>> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of
>> Jiamei Xie
>> Sent: 2022年3月17日 17:17
>> To: Ross Lagerwall <ross.lagerwall@citrix.com>; Bjoern Doebel
>> <doebel@amazon.de>; xen-devel@lists.xenproject.org
>> Cc: Michael Kurth <mku@amazon.de>; Martin Pohlack
>> <mpohlack@amazon.de>; Roger Pau Monne <roger.pau@citrix.com>;
>> Andrew Cooper <Andrew.Cooper3@citrix.com>; Konrad Rzeszutek Wilk
>> <konrad.wilk@oracle.com>
>> Subject: RE: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-
>> enhanced functions
>>
>> Hi Bjoern,
>>
>>> -----Original Message-----
>>> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of
>>> Ross Lagerwall
>>> Sent: 2022年3月10日 1:12
>>> To: Bjoern Doebel <doebel@amazon.de>; xen-devel@lists.xenproject.org
>>> Cc: Michael Kurth <mku@amazon.de>; Martin Pohlack
>>> <mpohlack@amazon.de>; Roger Pau Monne <roger.pau@citrix.com>;
>>> Andrew Cooper <Andrew.Cooper3@citrix.com>; Konrad Rzeszutek Wilk
>>> <konrad.wilk@oracle.com>
>>> Subject: Re: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-
>>> enhanced functions
>>>
>>>> From: Bjoern Doebel <doebel@amazon.de>
>>>> Sent: Wednesday, March 9, 2022 2:53 PM
>>>> To: xen-devel@lists.xenproject.org <xen-devel@lists.xenproject.org>
>>>> Cc: Michael Kurth <mku@amazon.de>; Martin Pohlack
>>> <mpohlack@amazon.de>; Roger Pau Monne <roger.pau@citrix.com>;
>>> Andrew Cooper <Andrew.Cooper3@citrix.com>; Bjoern Doebel
>>> <doebel@amazon.de>; Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>;
>>> Ross Lagerwall <ross.lagerwall@citrix.com>
>>>> Subject: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-
>>> enhanced functions
>>>>
>>>> Xen enabled CET for supporting architectures. The control flow aspect of
>>>> CET expects functions that can be called indirectly (i.e., via function
>>>> pointers) to start with an ENDBR64 instruction. Otherwise a control flow
>>>> exception is raised.
>>>>
>>>> This expectation breaks livepatching flows because we patch functions by
>>>> overwriting their first 5 bytes with a JMP + <offset>, thus breaking the
>>>> ENDBR64. We fix this by checking the start of a patched function for
>>>> being ENDBR64. In the positive case we move the livepatch JMP to start
>>>> behind the ENDBR64 instruction.
>>>>
>>>> To avoid having to guess the ENDBR64 offset again on patch reversal
>>>> (which might race with other mechanisms adding/removing ENDBR
>>>> dynamically), use the livepatch metadata to store the computed offset
>>>> along with the saved bytes of the overwritten function.
>>>>
>>>> Signed-off-by: Bjoern Doebel <doebel@amazon.de>
>>>> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>>> CC: Ross Lagerwall <ross.lagerwall@citrix.com>
>>>
>>> Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
>>
>> Tested-by: Jiamei xie <jiamei.xie@arm.com>
>>
>> Cheers,
>> Jiamei
> Sorry I forgot to add the scope I tested in last email. I tested it on armv8a. It worked fine and didn't break arm.
> Tested-by: Jiamei xie <jiamei.xie@arm.com>
Thanks Jiamei!
As Jan already pointed out there's a v6 patch out already. It is only
cosmetically different from this one. Unless you insist, I'd not roll a
v7 only to add this tag?
Bjoern
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-03-17 13:11 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-09 14:52 [PATCH v5 1/2] Livepatch: resolve old address before function verification Bjoern Doebel
2022-03-09 14:53 ` [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-enhanced functions Bjoern Doebel
2022-03-09 15:14 ` Jan Beulich
2022-03-09 17:12 ` Ross Lagerwall
2022-03-17 9:17 ` Jiamei Xie
2022-03-17 10:00 ` Jiamei Xie
2022-03-17 10:11 ` Jan Beulich
2022-03-17 13:10 ` Doebel, Bjoern
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.