From nobody Sun Nov 24 16:24:53 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass(p=reject dis=none) header.from=cloud.com ARC-Seal: i=1; a=rsa-sha256; t=1727448543; cv=none; d=zohomail.com; s=zohoarc; b=FRRTZDxgjihhqXlCb3q0ogMEtT0ySvU913uTlkwWEkz0Atp6uDgs93WI+vzSMAfBHxVpb7JDcHpDI1wyicJlEDeuNdxMHI8buWAtWW3NeyZssLhuwFypnLqTwxzzlzlpdFD22xV5ulQMXgDwO1P+i3wJniFJO9FmBEk/P4txkis= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1727448543; h=Content-Type:Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:Subject:To:To:Message-Id:Reply-To; bh=ZeXnWGkSfJrKVWToCCiBwAKBXIRPNqewnadfBa2N6OY=; b=WW5OKG/3LHwaFC0DFk70ZBWoYu5Bsimtc5VMGOUrpDw6mZ87RLG+GslfwpP73XqjSdCRMNKBfV2RPCs7fN7orwtZtVvbSJz1vpGTe79b779ymn7+m2awHDXyKCWeATCk/GnrPcK9xvAO24yeUqcLRYhwdJdzWg//ym0+dSkbr7I= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass header.from= (p=reject dis=none) Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1727448543246535.217863444416; Fri, 27 Sep 2024 07:49:03 -0700 (PDT) Received: from list by lists.xenproject.org with outflank-mailman.806340.1217665 (Exim 4.92) (envelope-from ) id 1suCGh-0005Pm-Sc; Fri, 27 Sep 2024 14:48:27 +0000 Received: by outflank-mailman (output) from mailman id 806340.1217665; Fri, 27 Sep 2024 14:48:27 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1suCGh-0005Pf-PP; Fri, 27 Sep 2024 14:48:27 +0000 Received: by outflank-mailman (input) for mailman id 806340; Fri, 27 Sep 2024 14:48:27 +0000 Received: from se1-gles-flk1-in.inumbo.com ([94.247.172.50] helo=se1-gles-flk1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1suCGh-0005PZ-5m for xen-devel@lists.xenproject.org; Fri, 27 Sep 2024 14:48:27 +0000 Received: from mail-lf1-x12c.google.com (mail-lf1-x12c.google.com [2a00:1450:4864:20::12c]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id 8ca16cd5-7cdf-11ef-99a2-01e77a169b0f; Fri, 27 Sep 2024 16:48:25 +0200 (CEST) Received: by mail-lf1-x12c.google.com with SMTP id 2adb3069b0e04-5398b589032so306206e87.1 for ; Fri, 27 Sep 2024 07:48:25 -0700 (PDT) Received: from mewpvdipd1023.corp.cloud.com ([52.174.59.202]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a93c27c58e1sm142625666b.58.2024.09.27.07.48.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 27 Sep 2024 07:48:23 -0700 (PDT) X-Outflank-Mailman: Message body and most headers restored to incoming version X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 8ca16cd5-7cdf-11ef-99a2-01e77a169b0f DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cloud.com; s=cloud; t=1727448504; x=1728053304; darn=lists.xenproject.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=ZeXnWGkSfJrKVWToCCiBwAKBXIRPNqewnadfBa2N6OY=; b=MZA33gHiYxfTommSsCB/6Q8QQpCC4Zuqyk0u19j9OpI+NvelUD2nDPN0kOMDtp/OIm nVeZkjS5DHmGnR3Bdlhbb8pHl+A1JyppMSYLvCwP4b7gMy+hkRWLjf2XcMzvJVReXurq Hv5ULVcEVsI8NAQjzmjAclT3zsl7XzynVIHEs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727448504; x=1728053304; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=ZeXnWGkSfJrKVWToCCiBwAKBXIRPNqewnadfBa2N6OY=; b=J18uxtZoOg21xIwnYjuMM3WVzi6QtogbwcWXH2NOkC9fxfao3HO5jivExnuNKOXLka IV0nOYrzv+QYQAhzMxgmZb/3k4twkK0eBlBryZnPYHCoB3ITeXk0rTCnZbG3qY0Pcext oSVHRo1W3vZeHSDwe5XmkLUXOSoQhxto8WnuCC4vA5MALl5zzspYQF2jkLyZ36MvLWVt W3bsCW4/OuXFTVo1wUzv+OR8zHyMKMzKtGVnJh0i+1oEkME/9LzcyJ4ILVbHchwUBAec ri1OwhE/S5//qjlWg42VHvkb9qXr8kGPZU2wvFlFzuL6FTHhAkA7ZNJ0jQ5O4YnkHJOt Wvmg== X-Gm-Message-State: AOJu0YzJzkM6ozjSxfCXIMUoGPLiZ6sf5XuVLdBmFmlGrofrslgx5Tb2 rWXdMbTvthmMP4YQrQKKiknpVwglEduI2ZQ2/4luGAR+84647qcDmdPkwNvUz26uzVCJ70Tg/bt U5Kg= X-Google-Smtp-Source: AGHT+IGB8c+q79IHtbrG+w/LfUZmxeiwAuteEISMaV0jlA8DGDvcUBWFl49PepHB7YAlYK0KbpsSMQ== X-Received: by 2002:a05:6512:4025:b0:52c:d628:c77c with SMTP id 2adb3069b0e04-5389fc7d15bmr2906265e87.43.1727448504102; Fri, 27 Sep 2024 07:48:24 -0700 (PDT) From: Alejandro Vallejo To: Xen-devel Cc: Alejandro Vallejo , Jan Beulich , Andrew Cooper , =?UTF-8?q?Roger=20Pau=20Monn=C3=A9?= Subject: [PATCH v3-resend] x86/traps: Re-enable interrupts after reading cr2 in the #PF handler Date: Fri, 27 Sep 2024 15:48:20 +0100 Message-ID: <20240927144820.300553-1-alejandro.vallejo@cloud.com> X-Mailer: git-send-email 2.46.0 MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @cloud.com) X-ZM-MESSAGEID: 1727448544563116600 Hitting a page fault clobbers %cr2, so if a page fault is handled while handling a previous page fault then %cr2 will hold the address of the latter fault rather than the former. In particular, if a debug key handler happens to trigger during #PF and before %cr2 is read, and that handler itself encounters a #PF, then %cr2 will be corrupt for the outer #PF handler. This patch makes the page fault path delay re-enabling IRQs until %cr2 has been read in order to ensure it stays consistent. A similar argument holds in additional cases, but they happen to be safe: * %dr6 inside #DB: Safe because IST exceptions don't re-enable IRQs. * MSR_XFD_ERR inside #NM: Safe because AMX isn't used in #NM handler. While in the area, remove redundant q suffix to a movq in entry.S and the space after the comma. Fixes: a4cd20a19073 ("[XEN] 'd' key dumps both host and guest state.") Signed-off-by: Alejandro Vallejo Acked-by: Roger Pau Monn=C3=A9 --- v3: * s/dispatch_handlers/dispatch_exceptions/ * Updated commit message, spelling out the state of #DB and #NM, and state an existing race with debug keys. --- xen/arch/x86/traps.c | 8 ++++++++ xen/arch/x86/x86_64/entry.S | 20 ++++++++++++++++---- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index 708136f62558..a9c2c607eb08 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -1600,6 +1600,14 @@ void asmlinkage do_page_fault(struct cpu_user_regs *= regs) =20 addr =3D read_cr2(); =20 + /* + * Don't re-enable interrupts if we were running an IRQ-off region when + * we hit the page fault, or we'll break that code. + */ + ASSERT(!local_irq_is_enabled()); + if ( regs->flags & X86_EFLAGS_IF ) + local_irq_enable(); + /* fixup_page_fault() might change regs->error_code, so cache it here.= */ error_code =3D regs->error_code; =20 diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S index b8482de8ee5b..9b0cdb76408b 100644 --- a/xen/arch/x86/x86_64/entry.S +++ b/xen/arch/x86/x86_64/entry.S @@ -844,9 +844,9 @@ handle_exception_saved: #elif !defined(CONFIG_PV) ASSERT_CONTEXT_IS_XEN #endif /* CONFIG_PV */ - sti -1: movq %rsp,%rdi - movzbl UREGS_entry_vector(%rsp),%eax +.Ldispatch_exceptions: + mov %rsp, %rdi + movzbl UREGS_entry_vector(%rsp), %eax #ifdef CONFIG_PERF_COUNTERS lea per_cpu__perfcounters(%rip), %rcx add STACK_CPUINFO_FIELD(per_cpu_offset)(%r14), %rcx @@ -866,7 +866,19 @@ handle_exception_saved: jmp .L_exn_dispatch_done; \ .L_ ## vec ## _done: =20 + /* + * IRQs kept off to derisk being hit by a nested interrupt before + * reading %cr2. Otherwise a page fault in the nested interrupt ha= ndler + * would corrupt %cr2. + */ DISPATCH(X86_EXC_PF, do_page_fault) + + /* Only re-enable IRQs if they were active before taking the fault= */ + testb $X86_EFLAGS_IF >> 8, UREGS_eflags + 1(%rsp) + jz 1f + sti +1: + DISPATCH(X86_EXC_GP, do_general_protection) DISPATCH(X86_EXC_UD, do_invalid_op) DISPATCH(X86_EXC_NM, do_device_not_available) @@ -911,7 +923,7 @@ exception_with_ints_disabled: movq %rsp,%rdi call search_pre_exception_table testq %rax,%rax # no fixup code for faulting EIP? - jz 1b + jz .Ldispatch_exceptions movq %rax,UREGS_rip(%rsp) # fixup regular stack =20 #ifdef CONFIG_XEN_SHSTK --=20 2.46.0