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

Re: [PATCH] x86/cet: Fix shskt manipulation error with BUGFRAME_{warn,run_fn}

  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Fri, 13 Aug 2021 16:00:41 +0200
  • 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-SenderADCheck; bh=ikvUIVVD7Jdc0rntdl62pdakrVq+rqyhTr09oVykQbI=; b=XLasQ62WdiKrp6X36n/9oHYuCwQIAY3Jtb9LBS+aLNrPziNS6W7ftBDrFjha/UP1Sl+7cbvTeXNNbDc0E0lPM5ecXnuecUOnBB/qw7mDBrOWgxq2yYnW4pXHnr44RTHn+2elagHNacgBGP2wCosev7iAxGwNLebjBGMyOS15IGWntPKQL79wjhuIS7r2GXk8tDBoPttjXyDmif5/XTq4/VBtykrm7mj6MAG2+iufrX4ICT8o0seh7Drs2oGq41Nrnact/dgAScnv0ZyXe2n6OTVLE8HcPrGbPX+SraW9gPpCUKlrrsnn374VymPAzxk3Cv2dRQ6lNtdqu44GmBSdYg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=L8WKAaesP/+38guImEMtuU3F9eRRuKTIyLxOdR1lrNDbuQ3relZYFDkbeWH8WPk0AH+potXqgzFIlDQErYRn3HSqr6xAG3d/F9VyDXymlz3MhLx6iGyBJwiCQjmxDXNbyv95+tLOhzb9xy2gbHACxwR73K4VpTANLjkDdziWCoJ9v+BapBvAOtU4zundZQYQrPzENS5/U3Ow8kng7BBShneFVHRj4J7550hd6mdG1oHcUxOJ70TU7yTRJdcVK2XsO2meys9CuTEDuaPvNLbgAo72OwDaqVpfv3fooYqFKbuFckeaxHbEK994ML2z4Ae/GntMCO2a3IpH5AQXj+3T0w==
  • Authentication-results: lists.xenproject.org; dkim=none (message not signed) header.d=none;lists.xenproject.org; dmarc=none action=none header.from=suse.com;
  • Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Fri, 13 Aug 2021 14:00:56 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 12.08.2021 19:03, Andrew Cooper wrote:
> This was a clear oversight in the original CET work.  The BUGFRAME_run_fn and
> BUGFRAME_warn paths update regs->rip without an equivlenet adjustment to the
> shadow stack, causes IRET to suffer #CP due to the mismatch.
> One subtle, and therefore fragile, aspect of extable_shstk_fixup() was that it
> required regs->rip to have its old value as a cross-check that the correct
> word in the shadow stack was being adjusted.
> Rework extable_shstk_fixup() into fixup_exception_return() which takes
> ownership of the update to both the regular and shadow stacks, ensuring that
> the regs->rip update is ordered suitably.
> Use the new fixup_exception_return() for BUGFRAME_run_fn and BUGFRAME_warn to
> ensure that the shadow stack is updated too.
> Fixes: 209fb9919b50 ("x86/extable: Adjust extable handling to be shadow stack 
> compatible")
> Reported-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

> I'm not a massive fan of the large ifdef area.  The logic could be rearranged
> to use IS_ENABLED(CONFIG_XEN_SHSTK) by indenting most of the function, but I
> can't see any way to drop the goto's, and this is certainly the least-invasive
> diff.

It's not really neat, but we've got worse code elsewhere.

I wonder whether gdb_arch_resume() and gdb_arch_write_reg() also
need some sort of similar adjustment.




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