Re: [PATCH] x86/hvm: Fix double free from vlapic_init() early error path

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.03.2021 15:49, Roger Pau Monné wrote:
> On Wed, Mar 31, 2021 at 02:31:25PM +0100, Andrew Cooper wrote:
>> @@ -1645,8 +1642,8 @@ void vlapic_destroy(struct vcpu *v)
>>      tasklet_kill(&vlapic->init_sipi.tasklet);
>>      TRACE_0D(TRC_HVM_EMUL_LAPIC_STOP_TIMER);
>>      destroy_periodic_time(&vlapic->pt);
>> -    unmap_domain_page_global(vlapic->regs);
>> -    free_domheap_page(vlapic->regs_page);
>> +    UNMAP_DOMAIN_PAGE_GLOBAL(vlapic->regs);
> 
> I think you need to check whether vlapic->regs_page is NULL here...
> 
>> +    FREE_DOMHEAP_PAGE(vlapic->regs_page);
>>  }
>>  
>>  /*
>> diff --git a/xen/include/xen/domain_page.h b/xen/include/xen/domain_page.h
>> index a182d33b67..0cb7f2aad3 100644
>> --- a/xen/include/xen/domain_page.h
>> +++ b/xen/include/xen/domain_page.h
>> @@ -77,4 +77,9 @@ static inline void unmap_domain_page_global(const void 
>> *va) {};
>>      (p) = NULL;                     \
>>  } while ( false )
>>  
>> +#define UNMAP_DOMAIN_PAGE_GLOBAL(p) do {   \
>> +    unmap_domain_page_global(p);           \
>> +    (p) = NULL;                            \
>> +} while ( false )
>> +
>>  #endif /* __XEN_DOMAIN_PAGE_H__ */
>> diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
>> index 667f9dac83..c274e2eac4 100644
>> --- a/xen/include/xen/mm.h
>> +++ b/xen/include/xen/mm.h
>> @@ -85,6 +85,12 @@ bool scrub_free_pages(void);
>>  } while ( false )
>>  #define FREE_XENHEAP_PAGE(p) FREE_XENHEAP_PAGES(p, 0)
>>  
>> +#define FREE_DOMHEAP_PAGES(p, o) do { \
>> +    free_domheap_pages(p, o);         \
> 
> ...as both unmap_domain_page_global and free_domheap_pages don't
> support being passed a NULL pointer.

Except that such checking would better go into the new macros,
alongside their clearing the pointers afterwards.

Jan