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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.03.2021 15:31, Andrew Cooper wrote:
> vlapic_init()'s caller calls vlapic_destroy() on error.  Therefore, the error
> path from __map_domain_page_global() failing would doubly free
> vlapic->regs_page.

I'm having difficulty seeing this. What I find at present is

    rc = vlapic_init(v);
    if ( rc != 0 ) /* teardown: vlapic_destroy */
        goto fail2;

and then

 fail3:
    vlapic_destroy(v);
 fail2:

Am I missing some important aspect?

> Rework vlapic_destroy() to be properly idempotent, introducing the necessary
> UNMAP_DOMAIN_PAGE_GLOBAL() and FREE_DOMHEAP_PAGE() wrappers.
> 
> Rearrange vlapic_init() to group all trivial initialisation, and leave all
> cleanup to the caller, in line with our longer term plans.

Cleanup functions becoming idempotent is what I understand is the
longer term plan. I didn't think this necessarily included leaving
cleanup after failure in a function to it caller(s). At least in the
general case I think it would be quite a bit better if functions
cleaned up after themselves - perhaps (using the example here) by
vlapic_init() calling vlapic_destroy() in such a case.

Jan