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

Re: [PATCH] xen/x86: Livepatch: support patching CET-enhanced functions


  • To: "Doebel, Bjoern" <doebel@xxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Mon, 7 Mar 2022 11:33:28 +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=GgbDyPoo8GQc0yETPpRZhJ3FGEFYKIVp1LBTWsa3RiE=; b=jUnDwqMLDTfZVYIVwIDO5caQqUgA4v0dFc+PJ/YVWcl+pKyTaHrkmkkUGICXQgEN16HDtViteZxM8fB1ki4KZ0j9P8FQo/DdP0gTceSttEWLW0U1nyEXNqnIHZ6+E35r/uoZSnHUy1m2x9HeIxP2juqoz1SRJC/UjVHdbwSwxgIzfC+sShMud40bi5tAn2HLoSNJnDA/p+SSQU7Xq7XFw0SJC7CmR7tzNd6L5e0yEzMe/DRn3MCM+7XUDdJG6LOmxSThbnVt6oJGs6DUVpsysGcph9i+1Beyywef7cVZNg65g/XpVepWlX/t8OiIa4ZUtu1v9EHT9AELR9hxyKUj1A==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=gS0ppJAKBPCl0MLyPeQWBsGDXwZn2C6iDL8CnmkyNmkSeOjjEVK5QZlhZ1vq+n/RSrxX76LgQhUm6jVhQVDwo/qZP8aBepRwuwg96vcBeXzKCGtz0TsdwreeWqFasEvCqwoCwPsLZi22zJqFv/uDKiScZUWrIE5t9Kl7gyPJD/44l6coglMu2NofwO5eU22mTl5nGIXfuGZ2anPhUFab0IOKlR1j2Uss9soodlyCxmfKnTnzpzSOkqHuJ6F+JyXOhnEA9ig1/bLGlslomCwAfX/uzybEOefx+DkffIs7HfI9Br5gGH75mD+8HRhCVKRLm1En5bIh0Jc9II2PH7XlFw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Michael Kurth <mku@xxxxxxxxx>, Martin Pohlack <mpohlack@xxxxxxxxx>, Roger Pau Monne <roger.pau@xxxxxxxxxx>, Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Mon, 07 Mar 2022 10:33:40 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 07.03.2022 11:17, Doebel, Bjoern wrote:
> On 07.03.22 10:37, Jan Beulich wrote:
>> On 07.03.2022 10:17, Bjoern Doebel wrote:
>>> --- a/xen/arch/x86/livepatch.c
>>> +++ b/xen/arch/x86/livepatch.c
>>> @@ -14,11 +14,28 @@
>>>   #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>
>>>   #include <asm/setup.h>
>>>
>>> +/*
>>> + * 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).
>>> + *
>>> + * We do so by making use of the existing opaque metadata area. We use its 
>>> first 4 bytes
>>> + * to track the offset into the function used for patching and the 
>>> remainder of the data
>>> + * to store overwritten code bytes.
>>> + */
>>
>> Style: Line length (also elsewhere).
>>
>>> +struct __packed x86_livepatch_meta {
>>> +    int32_t patch_offset;
>>
>> I was first going to suggest to use plain int here to comply with
>> ./CODING_STYLE, but it looks like int8_t or uint8_t will do, leaving
>> more space for the insn. I'm also not convinced you really need the
>> __packed annotation. Furthermore, to avoid the need for casts,
>> simply using the ->opaque[] directly would look to be an option then
>> (with suitable #define-s to identify the two parts).
> 
> * Agree on the uint8_t.
> * The __packed was mostly to be really sure there's no added padding.
>    Likely will be needed once I change the type for the offset.
> * I think using an explicit struct is more readable+comprehensible than
>    magically defining macros to reinterpret the opaque[] array, so I'd
>    prefer that. No hard feelings, though.

While I agree on the readable/comprehensible aspect, casts - in particular
such as used here - come with a risk. But it'll be up to the maintainers
to judge anyway.

Jan




 


Rackspace

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