linux-kernel - Re: [PATCH v12 09/14] x86/sgx: Implement async reclamation for cgroup

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: <e8d076fb097774f1f0fe3365883e6cf5a823fc4f.camel@intel.com>
Date: Mon, 22 Apr 2024 00:22:27 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "Mehta, Sohil" <sohil.mehta@...el.com>, "mingo@...hat.com"
	<mingo@...hat.com>, "jarkko@...nel.org" <jarkko@...nel.org>, "x86@...nel.org"
	<x86@...nel.org>, "dave.hansen@...ux.intel.com"
	<dave.hansen@...ux.intel.com>, "cgroups@...r.kernel.org"
	<cgroups@...r.kernel.org>, "hpa@...or.com" <hpa@...or.com>,
	"tim.c.chen@...ux.intel.com" <tim.c.chen@...ux.intel.com>,
	"linux-sgx@...r.kernel.org" <linux-sgx@...r.kernel.org>, "mkoutny@...e.com"
	<mkoutny@...e.com>, "haitao.huang@...ux.intel.com"
	<haitao.huang@...ux.intel.com>, "tglx@...utronix.de" <tglx@...utronix.de>,
	"tj@...nel.org" <tj@...nel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "bp@...en8.de" <bp@...en8.de>
CC: "mikko.ylinen@...ux.intel.com" <mikko.ylinen@...ux.intel.com>,
	"seanjc@...gle.com" <seanjc@...gle.com>, "anakrish@...rosoft.com"
	<anakrish@...rosoft.com>, "Zhang, Bo" <zhanb@...rosoft.com>,
	"kristen@...ux.intel.com" <kristen@...ux.intel.com>, "yangjie@...rosoft.com"
	<yangjie@...rosoft.com>, "Li, Zhiquan1" <zhiquan1.li@...el.com>,
	"chrisyan@...rosoft.com" <chrisyan@...rosoft.com>
Subject: Re: [PATCH v12 09/14] x86/sgx: Implement async reclamation for cgroup

On Fri, 2024-04-19 at 20:14 -0500, Haitao Huang wrote:
> > > I think we can add support for "sgx_cgroup=disabled" in future if indeed
> > > needed. But just for init failure, no?
> > > 
> > 
> > It's not about the commandline, which we can add in the future when
> > needed.  It's about we need to have a way to handle SGX cgroup being
> > disabled at boot time nicely, because we already have a case where we  
> > need
> > to do so.
> > 
> > Your approach looks half-way to me, and is not future extendible.  If we
> > choose to do it, do it right -- that is, we need a way to disable it
> > completely in both kernel and userspace so that userspace won't be able  
> > to
> > see it.
> 
> That would need more changes in misc cgroup implementation to support  
> sgx-disable. Right now misc does not have separate files for different  
> resource types. So we can only block echo "sgx_epc..." to those interface  
> files, can't really make files not visible.

"won't be able to see" I mean "only for SGX EPC resource", but not the
control files for the entire MISC cgroup.

I replied at the beginning of the previous reply:

"
Given SGX EPC is just one type of MISC cgroup resources, we cannot just 
disable MISC cgroup as a whole.
"

You just need to set the SGX EPC "capacity" to 0 to disable SGX EPC.  See
the comment of @misc_res_capacity:

 * Miscellaneous resources capacity for the entire machine. 0 capacity
 * means resource is not initialized or not present in the host.

And "blocking echo sgx_epc ... to those control files" is already
sufficient for the purpose of not exposing SGX EPC to userspace, correct?

E.g., if SGX cgroup is enabled, you can see below when you read "max":

 # cat /sys/fs/cgroup/my_group/misc.max
 # <resource1> <max1>
   sgx_epc ...
   ...

Otherwise you won't be able to see "sgx_epc":

 # cat /sys/fs/cgroup/my_group/misc.max
 # <resource1> <max1>
   ...

And when you try to write the "max" for "sgx_epc", you will hit error:

 # echo "sgx_epc 100" > /sys/fs/cgroup/my_group/misc.max
 # ... echo: write error: Invalid argument

The above applies to all the control files.  To me this is pretty much
means "SGX EPC is disabled" or "not supported" for userspace. 

Am I missing anything?