linux-kernel - RE: [PATCH v9 2/6] x86/sgx: Introduce a counter to count the sgx_(vepc

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 for Android: free password hash cracker in your pocket

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

Message-ID: <DM8PR11MB575050AECA6A18D871750F08E75EA@DM8PR11MB5750.namprd11.prod.outlook.com>
Date: Thu, 24 Jul 2025 12:33:48 +0000
From: "Reshetova, Elena" <elena.reshetova@...el.com>
To: "Huang, Kai" <kai.huang@...el.com>, "Hansen, Dave" <dave.hansen@...el.com>
CC: "seanjc@...gle.com" <seanjc@...gle.com>, "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>, "linux-sgx@...r.kernel.org"
	<linux-sgx@...r.kernel.org>, "Raynor, Scott" <scott.raynor@...el.com>
Subject: RE: [PATCH v9 2/6] x86/sgx: Introduce a counter to count the
 sgx_(vepc_)open()

> -----Original Message-----
> From: Huang, Kai <kai.huang@...el.com>
> Sent: Thursday, July 24, 2025 1:25 PM
> To: Reshetova, Elena <elena.reshetova@...el.com>; Hansen, Dave
> <dave.hansen@...el.com>
> Cc: seanjc@...gle.com; 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>; linux-sgx@...r.kernel.org; Raynor, Scott
> <scott.raynor@...el.com>
> Subject: Re: [PATCH v9 2/6] x86/sgx: Introduce a counter to count the
> sgx_(vepc_)open()

Thank you very much for your review Kai!

> 
> 
> >
> > +static int sgx_open(struct inode *inode, struct file *file)
> > +{
> > +	int ret;
> > +
> > +	ret = sgx_inc_usage_count();
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = __sgx_open(inode, file);
> > +	if (ret) {
> > +		sgx_dec_usage_count();
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static int sgx_release(struct inode *inode, struct file *file)
> >  {
> >  	struct sgx_encl *encl = file->private_data;
> > @@ -126,7 +143,7 @@ static long sgx_compat_ioctl(struct file *filep,
> unsigned int cmd,
> >
> >  static const struct file_operations sgx_encl_fops = {
> >  	.owner			= THIS_MODULE,
> > -	.open			= __sgx_open,
> > +	.open			= sgx_open,
> 
> If you merge the first patch to this one, you can avoid such chunk in the
> diff.

Yes, agree, I would have likely squashed whole this series into one patch,
but in this case I followed Jarkko's suggestion to do renaming of the
functions in the separate patch. 

> 
> In fact, I think merging the first patch to this one makes sense because
> __sgx_open() only makes sense when you have sgx_inc_usage_count().

Yes, agree, but again this would be against the suggestion I got previously.

> 
> [...]
> 
> >
> > +/* Counter to count the active SGX users */
> > +static int __maybe_unused sgx_usage_count;
> 
> As replied to the patch 6, I think you can just introduce this variable in
> that patch.

Yes, now that I dropped the sgx_usage_count fully
I guess it can be also defined in patch 6, albeit it was a bit
more logical imo to have it defined as unused already here
since we are introducing counting primitives. 

> 
> > +
> > +int sgx_inc_usage_count(void)
> > +{
> > +	return 0;
> > +}
> > +
> > +void sgx_dec_usage_count(void)
> > +{
> > +	return;
> > +}
> > +
> >
> 
> [...]
> 
> > @@ -265,6 +266,7 @@ static int __sgx_vepc_open(struct inode *inode,
> struct file *file)
> >  	vepc = kzalloc(sizeof(struct sgx_vepc), GFP_KERNEL);
> >  	if (!vepc)
> >  		return -ENOMEM;
> > +
> 
> Unintended change?

Ups, yes, missed this one, will fix.

> 
> >  	mutex_init(&vepc->lock);
> >  	xa_init(&vepc->page_array);
> >
> > @@ -273,6 +275,23 @@ static int __sgx_vepc_open(struct inode *inode,
> struct file *file)
> >  	return 0;
> >  }
> >
> > +static int sgx_vepc_open(struct inode *inode, struct file *file)
> > +{
> > +	int ret;
> > +
> > +	ret = sgx_inc_usage_count();
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret =  __sgx_vepc_open(inode, file);
> > +	if (ret) {
> > +		sgx_dec_usage_count();
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static long sgx_vepc_ioctl(struct file *file,
> >  			   unsigned int cmd, unsigned long arg)
> >  {
> > @@ -291,7 +310,7 @@ static long sgx_vepc_ioctl(struct file *file,
> >
> >  static const struct file_operations sgx_vepc_fops = {
> >  	.owner		= THIS_MODULE,
> > -	.open		= __sgx_vepc_open,
> > +	.open		= sgx_vepc_open,
> 
> Ditto to sgx_open().

Yes, if patches are merged, this would go away.
Jarkko, are ok with merging or do you still believe it
it better to have it as separate patches? 

Best Regards,
Elena.