linux-kernel - Re: [PATCH v3 02/21] x86/virt/tdx: Detect TDX private KeyIDs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <19ef56ed7852a2097125a588b124e4598ab26e91.camel@intel.com>
Date:   Tue, 19 Apr 2022 22:07:37 +1200
From:   Kai Huang <kai.huang@...el.com>
To:     Sathyanarayanan Kuppuswamy 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Cc:     seanjc@...gle.com, pbonzini@...hat.com, dave.hansen@...el.com,
        len.brown@...el.com, tony.luck@...el.com,
        rafael.j.wysocki@...el.com, reinette.chatre@...el.com,
        dan.j.williams@...el.com, peterz@...radead.org, ak@...ux.intel.com,
        kirill.shutemov@...ux.intel.com, isaku.yamahata@...el.com
Subject: Re: [PATCH v3 02/21] x86/virt/tdx: Detect TDX private KeyIDs

On Mon, 2022-04-18 at 22:42 -0700, Sathyanarayanan Kuppuswamy wrote:
> 
> On 4/5/22 9:49 PM, Kai Huang wrote:
> >   	detect_seam(c);
> > +	detect_tdx_keyids(c);
> 
> Do you want to add some return value to detect_seam() and not
> proceed if it fails?

I don't think this function should return.  However it may make sense to stop
detecting TDX KeyIDs when on some cpu SEAMRR is detected as not enabled (i.e. on
BSP when SEAMRR is not enabled by BIOS, or on any AP when there's BIOS bug that
BIOS doesn't configure SEAMRR consistently on all cpus).  The reason is TDX
KeyIDs can only be accessed by software runs in SEAM mode.  So if SEAMRR
configuration is broken, TDX KeyID configuration probably is broken too.

However detect_tdx_keyids() essentially only uses rdmsr_safe() to read some MSR,
so if there's any problem, rdmsr_safe() will catch it.  And SEAMRR is always
checked before doing any TDX related staff later, therefore in practice there
will be no problem.  But anyway I guess there's no harm to add additional SEAMRR
check in detect_tdx_keyids().  I'll think more on this.  Thanks.

> 
> In case if this function is going to be extended by future
> patch set, maybe do the same for detect_tdx_keyids()?
> 

I'd prefer to leaving this in current way until there's a real need.

-- 
Thanks,
-Kai