Re: [PATCH 3/4] selftests/sgx: Ensure enclave data available during debug print

Reinette Chatre
2022-01-28T19:41:03Z

Hi Dave,

On 1/28/2022 11:06 AM, Dave Hansen wrote:
> On 1/28/22 10:23, Reinette Chatre wrote:
>> In support of debugging the SGX tests print details from
>> the enclave and its memory mappings if any failure is encountered
>> during enclave loading.
>>
>> When a failure is encountered no data is printed because the
>> printing of the data is preceded by cleanup of the data.
>>
>> Move the data cleanup after the data print.
>
> Isn't it worse than that?
>
>> err:
>> - encl_delete(encl);
>> -
>> for (i = 0; i < encl->nr_segments; i++) {
>> seg = &encl->segment_tbl[i];
>
> encl_delete() does:
>
> free(encl->segment_tbl);
>
> but doesn't zero encl->nr_segments from what I can see. That seems like
> a use-after-free.

encl_delete() ends with:

memset(encl, 0, sizeof(*encl));

that will zero encl->nr_segments.

Even so, (after this change) the function to which this change belongs
flows as follows:

setup_test_encl()
{
...
if (!encl_load("test_encl.elf", encl, heap_size)) {
encl_delete(encl);
...
return false;
}

<=== /*
* At this point, because encl_load() succeeded,
* encl->segment_tbl and encl->nr_segments will
* be valid.
*/

/*
* Further initialization code, any of which can
* "goto err" on failure.
*/

err:
/* encl->segment_tbl and encl->nr_segments are valid for use */

...
encl_delete();
/* encl->segment_tbl and encl->nr_segments are NOT valid for use */
return false;
}

>
> Seems like we need to really run the selftest under valgrind.

Reinette