Re: [PATCH v3 1/7] x86/sgx: Provide indication of life-cycle of EPC pages

From: Jarkko Sakkinen
Date: Thu Jul 29 2021 - 20:34:04 EST


On Wed, Jul 28, 2021 at 03:12:03PM -0700, Dave Hansen wrote:
> On 7/28/21 1:46 PM, Tony Luck wrote:
> > +++ b/arch/x86/kernel/cpu/sgx/main.c
> > @@ -581,7 +581,7 @@ struct sgx_epc_page *sgx_alloc_epc_page(void *owner, bool reclaim)
> > for ( ; ; ) {
> > page = __sgx_alloc_epc_page();
> > if (!IS_ERR(page)) {
> > - page->owner = owner;
> > + page->owner = owner ? owner : page;
> > break;
> > }
>
> I'm a little worried about this.
>
> Let's say we get confused about the type of the page and dereference
> page->owner. If it's NULL, we get a nice oops. If it's a real, valid
> pointer, we get real valid memory back that we can scribble on.
>
> Wouldn't it be safer to do something like:
>
> page->owner = owner ? owner : (void *)-1;
>
> -1 is non-NULL, but also invalid, which makes it harder for us to poke
> ourselves in the eye.

Works for me.

/Jarkko