On Tue, 2021-06-15 at 08:39 -0700, Dave Hansen wrote: > On 6/15/21 3:16 AM, Kai Huang wrote: > > xa_destroy() needs to be called to destroy virtual EPC's page arra > > before calling kfree() to free the virtual EPC. Currently it is not > > calaled. Add the missing xa_destroy() to fix. > > Looks good Kai, thanks for fixing this. > > Could you please take a good look through the sgx_release() and the vpec > equivalent and see if anything else stands out as possibly being missed? I looked over. One potential issue is both sgx_encl and sgx_vepc have 'struct mutex lock' embedded, but mutex_destroy() is not called when they are released. However I am not sure whether this is worth fixing, since mutex_destroy() is empty unless CONFIG_DEBUG_MUTEXES is turned on (even with it turned on, mutex_destroy() doesn't do anything like freeing resources so in practice there should have no problem). Another thing is sgx_encl_release() doesn't explicitly call xa_erase() for each EPC page in encl->page_array when looping it over to free all EPC pages, but I think it is OK since xa_destroy() is called later which will destroy all xarray internal data structures. But I don't know internal implementation of xarray. > Also, is this the kind of thing that a simple open/add/close selftest > might have found? It might be useful but I don't think it can detect things like xa_destroy() being missing. > > Maybe we should beef up the selftests a bit. > > Acked-by: Dave Hansen <dave.hansen@xxxxxxxxx> Thank you!
