linux-kernel - RE: [PATCH v14 5/5] x86/sgx: Enable automatic SVN updates for SGX enclaves

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] [day] [month] [year] [list]

Message-ID: <DM8PR11MB575076B29223DA1C70EA35E2E731A@DM8PR11MB5750.namprd11.prod.outlook.com>
Date: Mon, 18 Aug 2025 05:41:58 +0000
From: "Reshetova, Elena" <elena.reshetova@...el.com>
To: "Huang, Kai" <kai.huang@...el.com>, "Hansen, Dave" <dave.hansen@...el.com>
CC: "linux-sgx@...r.kernel.org" <linux-sgx@...r.kernel.org>,
	"mingo@...nel.org" <mingo@...nel.org>, "Scarlata, Vincent R"
	<vincent.r.scarlata@...el.com>, "x86@...nel.org" <x86@...nel.org>,
	"jarkko@...nel.org" <jarkko@...nel.org>, "Annapurve, Vishal"
	<vannapurve@...gle.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "Mallick, Asit K" <asit.k.mallick@...el.com>,
	"Aktas, Erdem" <erdemaktas@...gle.com>, "Cai, Chong" <chongc@...gle.com>,
	"Bondarevska, Nataliia" <bondarn@...gle.com>, "seanjc@...gle.com"
	<seanjc@...gle.com>, "Raynor, Scott" <scott.raynor@...el.com>
Subject: RE: [PATCH v14 5/5] x86/sgx: Enable automatic SVN updates for SGX
 enclaves



> -----Original Message-----
> From: Huang, Kai <kai.huang@...el.com>
> Sent: Monday, August 18, 2025 8:04 AM
> To: Reshetova, Elena <elena.reshetova@...el.com>; Hansen, Dave
> <dave.hansen@...el.com>
> Cc: linux-sgx@...r.kernel.org; mingo@...nel.org; Scarlata, Vincent R
> <vincent.r.scarlata@...el.com>; x86@...nel.org; jarkko@...nel.org;
> Annapurve, Vishal <vannapurve@...gle.com>; linux-kernel@...r.kernel.org;
> Mallick, Asit K <asit.k.mallick@...el.com>; Aktas, Erdem
> <erdemaktas@...gle.com>; Cai, Chong <chongc@...gle.com>; Bondarevska,
> Nataliia <bondarn@...gle.com>; seanjc@...gle.com; Raynor, Scott
> <scott.raynor@...el.com>
> Subject: Re: [PATCH v14 5/5] x86/sgx: Enable automatic SVN updates for SGX
> enclaves
> 
> > > > >
> > > > >  void sgx_dec_usage_count(void)
> > > > >  {
> > > > > -	return;
> > > > > +	sgx_usage_count--;
> > > > >  }
> > > >
> > > > How is a plain int-- safe?
> > > >
> > > > Where's the locking?
> > >
> > > Sorry I missed this during review too.
> >
> > My line of thinking went that we don't actually
> > care or act on decrement (it is not a true ref counter)
> > and therefore, races here are ok. What I forgot is that
> > we loose basic atomicity also with plain int ((
> >
> > I would personally like to go back to atomic (this is
> > what it is exactly for), but I can also just add another
> > mutex here. Preferences?
> 
> You don't need another mutex AFAICT, just use the one you already have.
> 
> The problem of the raw 'count--' is it is not multiple threads safe, e.g.,
> IIUC, you could effectively lose one or more subtractions here, leading to
> counter never reaching to 0.

Yes, this is what I call atomicity. 

> 
> From the perspective of functionality, to me there's no difference between
> mutex vs atomic_t, so either would be fine.  But as shown in your v7 [*],
> it appears atomic_t version is still slightly more complicated than the
> mutex.

Yes, but it was because originally we tried to avoid mutex/lock in
fast (non-zero increment part). If we drop this requirement and condition
everything with the mutex (as now), then we can go back to simpler handling
via atomic in inc() and in dec() it would be simple atomic_dec(). Smth like this:

int sgx_inc_usage_count(void)
{
	int ret;

	guard(mutex)(&sgx_svn_lock);

	if (atomic64_read(&sgx_usage_count)==0) {
		ret = sgx_update_svn();
		if (ret)
			return ret;
	}

	atomic64_inc(&sgx_usage_count);

 	return 0;
}

void sgx_dec_usage_count(void)
{
	atomic64_dec(&sgx_usage_count);
}

> So since we are already here, I would say just use the mutex:
> 
> void sgx_dec_usage_count(void)
> {
> 	guard(mutex)(&sgx_svn_lock);
> 	sgx_usage_count--;
> }
> 
> Am I missing anything?

The mutex just seems such an overkill for plain atomicity requirement here
in dec(). But sure, I can send the next version with mutex. 

Best Regards,
Elena.