linux-kernel - RE: [PATCH v2 2/2] x86/sgx: Implement EUPDATESVN and opportunistically call it during first EPC page alloc

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: <DM8PR11MB575087BDAFA223EDCE9EC69EE7AF2@DM8PR11MB5750.namprd11.prod.outlook.com>
Date: Wed, 2 Apr 2025 13:11:25 +0000
From: "Reshetova, Elena" <elena.reshetova@...el.com>
To: Jarkko Sakkinen <jarkko@...nel.org>, "Annapurve, Vishal"
	<vannapurve@...gle.com>
CC: "Hansen, Dave" <dave.hansen@...el.com>, "linux-sgx@...r.kernel.org"
	<linux-sgx@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "x86@...nel.org" <x86@...nel.org>, "Mallick,
 Asit K" <asit.k.mallick@...el.com>, "Scarlata, Vincent R"
	<vincent.r.scarlata@...el.com>, "Cai, Chong" <chongc@...gle.com>, "Aktas,
 Erdem" <erdemaktas@...gle.com>, "dionnaglaze@...gle.com"
	<dionnaglaze@...gle.com>, "bondarn@...gle.com" <bondarn@...gle.com>, "Raynor,
 Scott" <scott.raynor@...el.com>
Subject: RE: [PATCH v2 2/2] x86/sgx: Implement EUPDATESVN and
 opportunistically call it during first EPC page alloc

> On Tue, Apr 01, 2025 at 09:35:33AM +0000, Reshetova, Elena wrote:
> > > > None of these exceptional conditions are fatal or present an
> > > > immediate danger to the system security. So, allowing the re-tries
> > > > seems logical in this case. In case re-tries also fail, the system
> > > > admin will have an option of gracefully shutting down all enclaves
> > > > and doing either a full reboot (if SVN is the only concern) or other
> > > > necessary actions like taking the physical node out of use, etc.
> > > >
> > > > Does this sound reasonable?
> > >
> > > Uknown error I don't think would hold that premise.
> >
> > True, unknown is an unknown ))
> > But unknown errors should not happen (per SGX spec), and the
> 
> Thus if for some reason unknown error code would be returned something
> would be horribly wrong (e.g. bad emulation of the opcode or who knows
> what) and thus it would make sense disable the driver if this happens.

Again, this is just unknown code with regards to this operation, EUDAPSTESVN,
and while yes it should not happen, there is no indicator that smth else is
definitely broken with the exception of EUPDATESVN functionality. 

> 
> Or maybe even BUG_ON() in this situation?

I think there is a high bar in the kernel for using BUG_ON() and broken
SGX code is likely not reaching this bar: the rest of kernel is definitely ok
in this situation so at max it should be WARN_ON(). 

> 
> > current SGX kernel code does not handle such errors in any other way
> > than notifying that operation failed for other ENCLS leaves. So, I don't
> > see why ENCLS[EUPDATESVN] should be different from existing behaviour?
> 
> While not disagreeing fully (it depends on call site), in some
> situations it is more difficult to take more preventive actions.
> 
> This is a situation where we know that there are *zero* EPC pages in
> traffic so it is relatively easy to stop the madness, isn't it?
> 
> I guess the best action would be make sgx_alloc_epc_page() return
> consistently -ENOMEM, if the unexpected happens.

But this would be very misleading imo. We do have memory, even page
allocation might function as normal in EPC, the only thing that is broken
can be EUPDATESVN functionality. Returning -ENOMEM in this case seems
wrong.

> 
> /* <- this
>  * Do not execute ENCLS[EUPDATESVN] if running in a VM since
>  * microcode updates are only meaningful to be applied on the host.
>  */
> 
> According to https://www.kernel.org/doc/Documentation/kernel-doc-nano-
> HOWTO.txt

Sure, will fix in the next version. Thanks for catching!

Best Regards,
Elena.


> 
> >
> > Best Regards,
> > Elena.
> >
> >
> 
> BR, Jarkko