Messages in this thread | | | Subject | Re: [PATCH v3 1/5] x86/sgx: Fix a resource leak in sgx_init() | From | Dave Hansen <> | Date | Wed, 3 Mar 2021 08:56:52 -0800 |
| |
On 3/3/21 7:03 AM, Jarkko Sakkinen wrote: > If sgx_page_cache_init() fails in the middle, a trivial return > statement causes unused memory and virtual address space reserved for > the EPC section, not freed. Fix this by using the same rollback, as > when sgx_page_reclaimer_init() fails. ... > @@ -708,8 +708,10 @@ static int __init sgx_init(void) > if (!cpu_feature_enabled(X86_FEATURE_SGX)) > return -ENODEV; > > - if (!sgx_page_cache_init()) > - return -ENOMEM; > + if (!sgx_page_cache_init()) { > + ret = -ENOMEM; > + goto err_page_cache; > + }
Currently, the only way sgx_page_cache_init() can fail is in the case that there are no sections:
if (!sgx_nr_epc_sections) { pr_err("There are zero EPC sections.\n"); return false; }
That only happened if all sgx_setup_epc_section() calls failed. sgx_setup_epc_section() never both allocates memory with vmalloc for section->pages *and* fails. If sgx_setup_epc_section() has a successful memremap() but a failed vmalloc(), it cleans up with memunmap().
In other words, I see how this _looks_ like a memory leak from sgx_init(), but I don't see an actual leak in practice.
Am I missing something?
| |