[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/8] x86/boot: Sanitise PKRU on boot
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
- Date: Thu, 12 Jan 2023 17:07:26 +0000
- Accept-language: en-GB, en-US
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.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=GsS+vEKyi7Jr/4YI9jEMDy8equIHYDeAWTzYCbpamrY=; b=f/vQtnhDpeCfp0swOI/RDsDsNP7aEyu11Wkq1k1TL42xHIo0Mg0aBAuqRRTeLBXVi7eIhf1CEq0yJpkpy8C8bM1WbFYRlxMCHWBRRIQxcArHm0dlQoNrxnKJvaMql9TrIxsIE84h7NFu/GgoaOgecaLahf4xGaFO83Ylcv3cI8vRrEliMdS1EG2etp9wLmOUMiAEfVopjC9sxlbbraDQZphAaFkSOaguSoHZk5hhAQrkDjPrrdncc9+sm09EkPh7pdCX+2Pg1RcmOwqw7v0isSL+Z5CldCy0CCZhL16cUNaZ6Xw6RYpRhG9/scG9fN25QAqYGLi8x+8pN9GBiSXoew==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=a26L1f1qeZR/LgFNerpF3BvEYpEUmnXN1F+Jk1WYp4Sd7GZ1ySkATuu8Or67O9eGsGbdQjuyWxV8cAJebTYkx5dmxGihZdUgxoWVccwi7qhfbjX2/35AIF8y2m9BgV8p+5MheLr2bYuFa92TZuDdAJ8rwXEDllPQrKn1MU3CYqNoLqNtVuOEu+uqHYFRaOrMzgDgDTfYGg0MnBRCsEN9/436/9RVEqJAT6CPY1G6+Pnox0e6RVRmvbb85zxN7RC2bKMe1NAc2k34JobUe9Rv0Y7iyO1WWwo77vY96ZXMChEzjuSBhnCYTnlwOIDzEVfrq3twRny6LpnvvWEcAIMXEg==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: Roger Pau Monne <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- Delivery-date: Thu, 12 Jan 2023 17:07:43 +0000
- Ironport-data: A9a23:LJ7ZdqOt9IIbmpDvrR2HlsFynXyQoLVcMsEvi/4bfWQNrUp01DYHn zMXXWmDOv+IMGSketB2PYyyp0xSupbSxtVnSQto+SlhQUwRpJueD7x1DKtS0wC6dZSfER09v 63yTvGacajYm1eF/k/F3oDJ9CU6jufQA+KmU4YoAwgpLSd8UiAtlBl/rOAwh49skLCRDhiE/ Nj/uKUzAnf8s9JPGj9Suv3rRC9H5qyo42tB5wZmP5ingXeF/5UrJMNHTU2OByOQrrl8RoaSW +vFxbelyWLVlz9F5gSNy+uTnuUiG9Y+DCDW4pZkc/HKbitq/0Te5p0TJvsEAXq7vh3S9zxHJ HehgrTrIeshFvWkdO3wyHC0GQkmVUFN0OevzXRSLaV/ZqAJGpfh66wGMa04AWEX0rgwKDtS9 9A6EykIcBqP1+OH5aiFV+Y506zPLOGzVG8ekldJ6GmFSNMZG9XESaiM4sJE1jAtgMwIBezZe 8cSdTtoalLHfgFLPVAUTpk5mY9EhFGmK2Ee9A3T+vFxujeIpOBy+OGF3N79U9qGX8hK2G2fo XrL5T/RCRAGLt2PjzGC9xpAg8eex3iqANpOSNVU8NZkuEeYm09NOiEnUFe1usuwuEfgVI5Af hl8Fi0G6PJaGFaQZtv3UgC8oXWElgUBQNcWGOo/gCmSzoLE7gDfAXILJhZRZdpjuMIoSDgC0 l6Sg8ivFTFpqKeSS3+W6vGTtzzaBMQOBWoLZCtBSBRf5dDm+N03lkiWEY0lF7OphNroHz222 yqNsCU1m7QUi4gMyrm/+lfExTmro/AlUzII2+keZUr9hisRWWJvT9XABYTzhRqYELukcw==
- Ironport-hdrordr: A9a23:DDkcS6uK3YRj40v6tAYBsn9w7skDstV00zEX/kB9WHVpm6yj+v xG/c5rsCMc7Qx6ZJhOo7+90cW7L080lqQFg7X5X43DYOCOggLBQL2KhbGI/9SKIVycygcy78 Zdm6gVMqyLMbB55/yKnTVRxbwbsaW6GKPDv5ag8590JzsaD52Jd21Ce36m+ksdfnggObMJUK Cyy+BgvDSadXEefq2AdwI4t7iqnaysqHr+CyR2fiIa1A==
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
- Thread-index: AQHZJReg9aEubjLXpkqfuIdyn2CTs66avfyAgABIhQA=
- Thread-topic: [PATCH v2 1/8] x86/boot: Sanitise PKRU on boot
On 12/01/2023 12:47 pm, Jan Beulich wrote:
> On 10.01.2023 18:18, Andrew Cooper wrote:
>> While the reset value of the register is 0, it might not be after kexec/etc.
>> If PKEY0.{WD,AD} have leaked in from an earlier context, construction of a PV
>> dom0 will explode.
>>
>> Sequencing wise, this must come after setting CR4.PKE, and before we touch
>> any
>> user mappings.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> ---
>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>> CC: Wei Liu <wl@xxxxxxx>
>>
>> For sequencing, it could also come after setting XCR0.PKRU too, but then we'd
>> need to construct an empty XSAVE area to XRSTOR from, and that would be even
>> more horrible to arrange.
> That would be ugly for other reasons as well, I think.
Yeah - I absolutely don't want to go down this route.
>
>> --- a/xen/arch/x86/cpu/common.c
>> +++ b/xen/arch/x86/cpu/common.c
>> @@ -936,6 +936,9 @@ void cpu_init(void)
>> write_debugreg(6, X86_DR6_DEFAULT);
>> write_debugreg(7, X86_DR7_DEFAULT);
>>
>> + if (cpu_has_pku)
>> + wrpkru(0);
> What about the BSP during S3 resume? Shouldn't we play safe there too, just
> in case?
Out of S3, I think it's reasonable to rely on proper reset values, and
for pkru, and any issues of it being "wrong" should be fixed when we
reload d0v0's XSAVE state.
That said, I'm wanting to try and merge parts of the boot and S3 paths
because we're finding no end of errors/oversights, not least because we
have no automated testing of S3 suspend/resume. Servers typically don't
implement it, and fixes either come from code inspection, or Qubes
noticing (which is absolutely better than nothing, but not a great
reflection on Xen).
But to merge these things, I first need to finish the work to make
microcode loading properly early, and then fix up some of the feature
detection paths, and cleanly separate feature detection from applying
the chosen configuration, at which point I hope the latter part will be
reusable on the S3 resume path.
I don't expect this work to happen imminently...
~Andrew
|
