linux-kernel - Re: [PATCH v3 01/28] x86/sgx: Store struct sgx

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, 17 Jul 2023 11:14:56 +0000
From:   "Jarkko Sakkinen" <jarkko@...nel.org>
To:     "Haitao Huang" <haitao.huang@...ux.intel.com>,
        <dave.hansen@...ux.intel.com>, <tj@...nel.org>,
        <linux-kernel@...r.kernel.org>, <linux-sgx@...r.kernel.org>,
        <cgroups@...r.kernel.org>, "Thomas Gleixner" <tglx@...utronix.de>,
        "Ingo Molnar" <mingo@...hat.com>, "Borislav Petkov" <bp@...en8.de>,
        <x86@...nel.org>, "H. Peter Anvin" <hpa@...or.com>
Cc:     <kai.huang@...el.com>, <reinette.chatre@...el.com>,
        <zhiquan1.li@...el.com>, <kristen@...ux.intel.com>,
        <seanjc@...gle.com>
Subject: Re: [PATCH v3 01/28] x86/sgx: Store struct sgx_encl when allocating
 new VA pages

On Wed Jul 12, 2023 at 11:01 PM UTC, Haitao Huang wrote:
> In a later patch, when a cgroup has exceeded the max capacity for EPC pages
> and there are no more Enclave EPC pages associated with the cgroup that can
> be reclaimed, the only pages still associated with an enclave will be the
> unreclaimable Version Array (VA) pages or SECS pages, and the entire
> enclave will need to be killed to free up those pages.
>
> Currently, given an enclave pointer it is easy to find the associated VA
> pages and free them, however, OOM killing an enclave based on cgroup limits
> will require examining a cgroup's unreclaimable page list, and finding an
> enclave given a SECS page or a VA page. This will require a backpointer
> from a page to an enclave, including for VA pages.
>
> When allocating new Version Array (VA) pages, pass the struct sgx_encl of
> the enclave that is allocating the page. sgx_alloc_epc_page() will store
> this value in the owner field of the struct sgx_epc_page.  In a later
> patch, VA pages will be placed in an unreclaimable queue, and then when the
> cgroup max limit is reached and there are no more reclaimable pages and the
> enclave must be OOM killed, all the VA pages associated with that enclave
> can be uncharged and freed.
>
> To avoid casting needed to access the two types of owners: sgx_encl for VA
> pages, sgx_encl_page for other pages, replace 'owner' field in sgx_epc_page
> with a union of the two types.

I think the action taken is correct but the reasoning is a bit
convoluted.

Why not instead put something like:

"Because struct sgx_epc_page instances of VA pages are not owned by an
sgx_encl_page instance in the first place, mark their owner as sgx_encl,
in order to make it reachable from the unreclaimable list."

The code change itself, and rest of the paragraphs do look reasonable.

BR, Jarkko