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]
Date: Mon, 22 Apr 2024 11:17:21 -0500
From: "Haitao Huang" <haitao.huang@...ux.intel.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>, "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>, "Huang, Kai"
 <kai.huang@...el.com>
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 Sun, 21 Apr 2024 19:22:27 -0500, Huang, Kai <kai.huang@...el.com> wrote:

> 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  
>> interfacefiles, 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.
> "
>
Sorry I missed this point. below.

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

IIUC I don't think the situation we have is either of those cases. For our  
case, resource is inited and present on the host but we have allocation  
error for sgx cgroup infra.

> 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.
>
You are right, capacity == 0 does block echoing max and users see an error  
if they do that. But 1) doubt you literately wanted "SGX EPC is disabled"  
and make it unsupported in this case, 2) even if we accept this is "sgx  
cgroup disabled" I don't see how it is much better user experience than  
current solution or really helps user better.

Also to implement this approach, as you mentioned, we need workaround the  
fact that misc_try_charge() fails when capacity set to zero, and adding  
code to return root always? So it seems like more workaround code to just  
make it work for a failing case no one really care much and end result is  
not really much better IMHO.

Thanks
Haitao
 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ