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

Message-ID: <c9a730751f34087bdde3ea63a8c0d3a70b8d84b5.camel@intel.com>
Date: Mon, 18 Aug 2025 05:03:40 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "Reshetova, Elena" <elena.reshetova@...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

> > > > 
> > > >  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.

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.

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?

[*]
https://lore.kernel.org/linux-sgx/20250711165212.1354943-1-elena.reshetova@intel.com/T/#me3d9ca942330039a59e2dd6e1d14b81c6670f87a