[PATCH v12 5/5] x86/sgx: Enable automatic SVN updates for SGX enclaves

Elena Reshetova posted 5 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH v12 5/5] x86/sgx: Enable automatic SVN updates for SGX enclaves
Posted by Elena Reshetova 1 month, 3 weeks ago
== Background ==

ENCLS[EUPDATESVN] is a new SGX instruction [1] which allows enclave
attestation to include information about updated microcode SVN without a
reboot. Before an EUPDATESVN operation can be successful, all SGX memory
(aka. EPC) must be marked as “unused” in the SGX hardware metadata
(aka.EPCM). This requirement ensures that no compromised enclave can
survive the EUPDATESVN procedure and provides an opportunity to generate
new cryptographic assets.

== Solution ==

Attempt to execute ENCLS[EUPDATESVN] every time the first file descriptor
is obtained via sgx_(vepc_)open(). In the most common case the microcode
SVN is already up-to-date, and the operation succeeds without updating SVN.

Note: while in such cases the underlying crypto assets are regenrated, it
does not affect enclaves' visible keys obtained via EGETKEY instruction.

If it fails with any other error code than SGX_INSUFFICIENT_ENTROPY, this
is considered unexpected and the *open() returns an error. This should not
happen in practice.

On contrary, SGX_INSUFFICIENT_ENTROPY might happen due to a pressure on the
system's DRNG (RDSEED) and therefore the *open() can be safely retried to
allow normal enclave operation.

[1] Runtime Microcode Updates with Intel Software Guard Extensions,
https://cdrdv2.intel.com/v1/dl/getContent/648682

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
---
 arch/x86/kernel/cpu/sgx/main.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index 829bcba77d41..39bff3488350 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -934,7 +934,7 @@ static int sgx_usage_count;
  *				entropy in RNG
  * %-EIO:		- Unexpected error, retries are not advisable
  */
-static int __maybe_unused sgx_update_svn(void)
+static int sgx_update_svn(void)
 {
 	int ret;
 
@@ -992,14 +992,22 @@ static int __maybe_unused sgx_update_svn(void)
 	return -EIO;
 }
 
+/* Mutex to ensure no concurrent EPC accesses during EUPDATESVN */
+static DEFINE_MUTEX(sgx_svn_lock);
+
 int sgx_inc_usage_count(void)
 {
+	guard(mutex)(&sgx_svn_lock);
+
+	if (sgx_usage_count++ == 0)
+		return sgx_update_svn();
+
 	return 0;
 }
 
 void sgx_dec_usage_count(void)
 {
-	return;
+	sgx_usage_count--;
 }
 
 static int __init sgx_init(void)
-- 
2.45.2

Re: [PATCH v12 5/5] x86/sgx: Enable automatic SVN updates for SGX enclaves
Posted by Huang, Kai 1 month, 3 weeks ago
>  
> +/* Mutex to ensure no concurrent EPC accesses during EUPDATESVN */
> +static DEFINE_MUTEX(sgx_svn_lock);
> +
>  int sgx_inc_usage_count(void)
>  {
> +	guard(mutex)(&sgx_svn_lock);
> +
> +	if (sgx_usage_count++ == 0)
> +		return sgx_update_svn();
> +

Hmm.. sorry for not noticing this before.. But I think we might have a
problem here since the sgx_usage_count is increased regardless of the
result of sgx_update_svn().

If sgx_update_svn() fails, it makes sgx_inc_usage_count() return error
too, so sgx_{vepc_}open() will fail and return immediately w/o calling 
sgx_dec_usage_count().

But the sgx_usage_count has been increased.

AFAICT when sgx_{vepc_}_open() fails, the sgx_{vepc_}release() is not
called, so sgx_dec_usage_count() is never called and sgx_usage_count
remains increased.

So when sgx_{vepc_}open() calls sgx_inc_usage_count() again, it will skip
calling sgx_update_svn(), and allow enclave/vEPC to be created
successfully, which just defeats the purpose.

So if I am not missing anything, I think we should only increase the count
when sgx_update_svn() returns success?

>  	return 0;
>  }
>  
>  void sgx_dec_usage_count(void)
>  {
> -	return;
> +	sgx_usage_count--;
>  }
>  
>  static int __init sgx_init(void)
RE: [PATCH v12 5/5] x86/sgx: Enable automatic SVN updates for SGX enclaves
Posted by Reshetova, Elena 1 month, 3 weeks ago

> -----Original Message-----
> From: Huang, Kai <kai.huang@intel.com>
> Sent: Monday, August 11, 2025 1:50 PM
> To: Reshetova, Elena <elena.reshetova@intel.com>; Hansen, Dave
> <dave.hansen@intel.com>
> Cc: seanjc@google.com; mingo@kernel.org; Scarlata, Vincent R
> <vincent.r.scarlata@intel.com>; x86@kernel.org; jarkko@kernel.org;
> Annapurve, Vishal <vannapurve@google.com>; linux-kernel@vger.kernel.org;
> Mallick, Asit K <asit.k.mallick@intel.com>; Aktas, Erdem
> <erdemaktas@google.com>; Cai, Chong <chongc@google.com>; Bondarevska,
> Nataliia <bondarn@google.com>; linux-sgx@vger.kernel.org; Raynor, Scott
> <scott.raynor@intel.com>
> Subject: Re: [PATCH v12 5/5] x86/sgx: Enable automatic SVN updates for SGX
> enclaves
> 
> 
> >
> > +/* Mutex to ensure no concurrent EPC accesses during EUPDATESVN */
> > +static DEFINE_MUTEX(sgx_svn_lock);
> > +
> >  int sgx_inc_usage_count(void)
> >  {
> > +	guard(mutex)(&sgx_svn_lock);
> > +
> > +	if (sgx_usage_count++ == 0)
> > +		return sgx_update_svn();
> > +
> 
> Hmm.. sorry for not noticing this before.. But I think we might have a
> problem here since the sgx_usage_count is increased regardless of the
> result of sgx_update_svn().
> 
> If sgx_update_svn() fails, it makes sgx_inc_usage_count() return error
> too, so sgx_{vepc_}open() will fail and return immediately w/o calling
> sgx_dec_usage_count().
> 
> But the sgx_usage_count has been increased.
> 
> AFAICT when sgx_{vepc_}_open() fails, the sgx_{vepc_}release() is not
> called, so sgx_dec_usage_count() is never called and sgx_usage_count
> remains increased.
> 
> So when sgx_{vepc_}open() calls sgx_inc_usage_count() again, it will skip
> calling sgx_update_svn(), and allow enclave/vEPC to be created
> successfully, which just defeats the purpose.
> 
> So if I am not missing anything, I think we should only increase the count
> when sgx_update_svn() returns success?

Yes, you are right, thanks for catching this! In past the atomic version of
this patch did exactly, but after I went into this simplified version of counting,
this angle got broken.
Will fix. 

Best Regards,
Elena.
Re: [PATCH v12 5/5] x86/sgx: Enable automatic SVN updates for SGX enclaves
Posted by Huang, Kai 1 month, 3 weeks ago
> > 
> > > 
> > > +/* Mutex to ensure no concurrent EPC accesses during EUPDATESVN */
> > > +static DEFINE_MUTEX(sgx_svn_lock);
> > > +
> > >  int sgx_inc_usage_count(void)
> > >  {
> > > +	guard(mutex)(&sgx_svn_lock);
> > > +
> > > +	if (sgx_usage_count++ == 0)
> > > +		return sgx_update_svn();
> > > +
> > 
> > Hmm.. sorry for not noticing this before.. But I think we might have a
> > problem here since the sgx_usage_count is increased regardless of the
> > result of sgx_update_svn().
> > 
> > If sgx_update_svn() fails, it makes sgx_inc_usage_count() return error
> > too, so sgx_{vepc_}open() will fail and return immediately w/o calling
> > sgx_dec_usage_count().
> > 
> > But the sgx_usage_count has been increased.
> > 
> > AFAICT when sgx_{vepc_}_open() fails, the sgx_{vepc_}release() is not
> > called, so sgx_dec_usage_count() is never called and sgx_usage_count
> > remains increased.
> > 
> > So when sgx_{vepc_}open() calls sgx_inc_usage_count() again, it will skip
> > calling sgx_update_svn(), and allow enclave/vEPC to be created
> > successfully, which just defeats the purpose.
> > 
> > So if I am not missing anything, I think we should only increase the count
> > when sgx_update_svn() returns success?
> 
> Yes, you are right, thanks for catching this! In past the atomic version of
> this patch did exactly, but after I went into this simplified version of counting,
> this angle got broken.
> Will fix. 

Btw, I noticed this when I was looking at:

	WARN(sgx_usage_count != 1, "...");

in patch 4 and wondering why it's not "!= 0".

Please don't forget to update that when needed.
RE: [PATCH v12 5/5] x86/sgx: Enable automatic SVN updates for SGX enclaves
Posted by Reshetova, Elena 1 month, 3 weeks ago

> -----Original Message-----
> From: Huang, Kai <kai.huang@intel.com>
> Sent: Monday, August 11, 2025 11:37 PM
> To: Reshetova, Elena <elena.reshetova@intel.com>; Hansen, Dave
> <dave.hansen@intel.com>
> Cc: linux-sgx@vger.kernel.org; mingo@kernel.org; Scarlata, Vincent R
> <vincent.r.scarlata@intel.com>; x86@kernel.org; jarkko@kernel.org;
> Annapurve, Vishal <vannapurve@google.com>; linux-kernel@vger.kernel.org;
> Mallick, Asit K <asit.k.mallick@intel.com>; Aktas, Erdem
> <erdemaktas@google.com>; Cai, Chong <chongc@google.com>; Bondarevska,
> Nataliia <bondarn@google.com>; seanjc@google.com; Raynor, Scott
> <scott.raynor@intel.com>
> Subject: Re: [PATCH v12 5/5] x86/sgx: Enable automatic SVN updates for SGX
> enclaves
> 
> > >
> > > >
> > > > +/* Mutex to ensure no concurrent EPC accesses during EUPDATESVN */
> > > > +static DEFINE_MUTEX(sgx_svn_lock);
> > > > +
> > > >  int sgx_inc_usage_count(void)
> > > >  {
> > > > +	guard(mutex)(&sgx_svn_lock);
> > > > +
> > > > +	if (sgx_usage_count++ == 0)
> > > > +		return sgx_update_svn();
> > > > +
> > >
> > > Hmm.. sorry for not noticing this before.. But I think we might have a
> > > problem here since the sgx_usage_count is increased regardless of the
> > > result of sgx_update_svn().
> > >
> > > If sgx_update_svn() fails, it makes sgx_inc_usage_count() return error
> > > too, so sgx_{vepc_}open() will fail and return immediately w/o calling
> > > sgx_dec_usage_count().
> > >
> > > But the sgx_usage_count has been increased.
> > >
> > > AFAICT when sgx_{vepc_}_open() fails, the sgx_{vepc_}release() is not
> > > called, so sgx_dec_usage_count() is never called and sgx_usage_count
> > > remains increased.
> > >
> > > So when sgx_{vepc_}open() calls sgx_inc_usage_count() again, it will skip
> > > calling sgx_update_svn(), and allow enclave/vEPC to be created
> > > successfully, which just defeats the purpose.
> > >
> > > So if I am not missing anything, I think we should only increase the count
> > > when sgx_update_svn() returns success?
> >
> > Yes, you are right, thanks for catching this! In past the atomic version of
> > this patch did exactly, but after I went into this simplified version of counting,
> > this angle got broken.
> > Will fix.
> 
> Btw, I noticed this when I was looking at:
> 
> 	WARN(sgx_usage_count != 1, "...");
> 
> in patch 4 and wondering why it's not "!= 0".
> 
> Please don't forget to update that when needed.

Yes, of course.