lkml.org 
[lkml]   [2022]   [Nov]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v7 03/20] x86/virt/tdx: Disable TDX if X2APIC is not enabled
    From


    On 11/21/22 1:44 AM, Huang, Kai wrote:
    > On Sun, 2022-11-20 at 19:51 -0800, Sathyanarayanan Kuppuswamy wrote:
    >>
    >> On 11/20/22 4:26 PM, Kai Huang wrote:
    >>> The MMIO/xAPIC interface has some problems, most notably the APIC LEAK
    >>
    >> "some problems" looks more generic. May be we can be specific here. Like
    >> it has security issues?
    >
    > It was quoted from below upstream commit id (I only kept the one that I quoted
    > to save space):

    Ok.

    >
    > commit b8d1d163604bd1e600b062fb00de5dc42baa355f (tag: x86_apic_for_v6.1_rc1,
    > tip/x86/apic)
    > Author: Daniel Sneddon <daniel.sneddon@linux.intel.com>
    > Date: Tue Aug 16 16:19:42 2022 -0700
    >
    > x86/apic: Don't disable x2APIC if locked
    >
    > ....
    >
    > The MMIO/xAPIC interface has some problems, most notably the APIC LEAK
    > [1]. This bug allows an attacker to use the APIC MMIO interface to
    > extract data from the SGX enclave.
    >
    > ....
    >
    > [1]: https://aepicleak.com/aepicleak.pdf
    >
    > Signed-off-by: Daniel Sneddon <daniel.sneddon@linux.intel.com>
    > Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
    > Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
    > Tested-by: Neelima Krishnan <neelima.krishnan@intel.com>
    > Link:
    > https://lkml.kernel.org/r/20220816231943.1152579-1-daniel.sneddon@linux.intel.com
    >
    >
    >>
    >>> [1]. This bug allows an attacker to use the APIC MMIO interface to
    >>> extract data from the SGX enclave.
    >>>
    >>> TDX is not immune from this either. Early check X2APIC and disable TDX
    >>> if X2APIC is not enabled, and make INTEL_TDX_HOST depend on X86_X2APIC.
    >>>
    >>> [1]: https://aepicleak.com/aepicleak.pdf
    >>>
    >>> Link: https://lore.kernel.org/lkml/d6ffb489-7024-ff74-bd2f-d1e06573bb82@intel.com/
    >>> Link: https://lore.kernel.org/lkml/ba80b303-31bf-d44a-b05d-5c0f83038798@intel.com/
    >>> Signed-off-by: Kai Huang <kai.huang@intel.com>
    >>> ---
    >>>
    >>> v6 -> v7:
    >>> - Changed to use "Link" for the two lore links to get rid of checkpatch
    >>> warning.
    >>>
    >>> ---
    >>> arch/x86/Kconfig | 1 +
    >>> arch/x86/virt/vmx/tdx/tdx.c | 11 +++++++++++
    >>> 2 files changed, 12 insertions(+)
    >>>
    >>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
    >>> index cced4ef3bfb2..dd333b46fafb 100644
    >>> --- a/arch/x86/Kconfig
    >>> +++ b/arch/x86/Kconfig
    >>> @@ -1958,6 +1958,7 @@ config INTEL_TDX_HOST
    >>> depends on CPU_SUP_INTEL
    >>> depends on X86_64
    >>> depends on KVM_INTEL
    >>> + depends on X86_X2APIC
    >>> help
    >>> Intel Trust Domain Extensions (TDX) protects guest VMs from malicious
    >>> host and certain physical attacks. This option enables necessary TDX
    >>> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
    >>> index 982d9c453b6b..8d943bdc8335 100644
    >>> --- a/arch/x86/virt/vmx/tdx/tdx.c
    >>> +++ b/arch/x86/virt/vmx/tdx/tdx.c
    >>> @@ -12,6 +12,7 @@
    >>> #include <linux/printk.h>
    >>> #include <asm/msr-index.h>
    >>> #include <asm/msr.h>
    >>> +#include <asm/apic.h>
    >>> #include <asm/tdx.h>
    >>> #include "tdx.h"
    >>>
    >>> @@ -81,6 +82,16 @@ static int __init tdx_init(void)
    >>> goto no_tdx;
    >>> }
    >>>
    >>> + /*
    >>> + * TDX requires X2APIC being enabled to prevent potential data
    >>> + * leak via APIC MMIO registers. Just disable TDX if not using
    >>> + * X2APIC.
    >>
    >> Remove the double space.
    >
    > Sorry which "double space"?

    Before Just disable. It looked like double space. Is it not?

    >
    >>
    >>> + */
    >>> + if (!x2apic_enabled()) {
    >>> + pr_info("Disable TDX as X2APIC is not enabled.\n");
    >>
    >> pr_warn()?
    >
    > Why?

    Since it is a failure case, I thought pr_warn would be better. It is up
    to you.

    >

    --
    Sathyanarayanan Kuppuswamy
    Linux Kernel Developer

    \
     
     \ /
      Last update: 2022-11-21 23:00    [W:5.261 / U:0.212 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site