Re: [Xen-devel] [PATCH] xen/x86: domain: Free all the pages associated to struct domain

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 22/01/2020 13:13, Julien Grall wrote:
> Hi Andrew,
>
> On 22/01/2020 12:52, Andrew Cooper wrote:
>> On 20/01/2020 14:31, Julien Grall wrote:
>>> From: Julien Grall <jgrall@xxxxxxxxxx>
>>>
>>> The structure domain may be bigger than a page size when lock profiling
>>> is enabled. However, the function free_domheap_struct will only free
>>> the
>>> first page.
>>>
>>> This is not a security issue because struct domain can only be bigger
>>> than a page size for lock profiling. The feature can only be selected
>>> in DEBUG and EXPERT mode.
>>>
>>> Fixes: 8916fcf4577 ("x86/domain: compile with lock_profile=y enabled")
>>> Reported-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
>>> Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
>>> ---
>>>   xen/arch/x86/domain.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
>>> index 28fefa1f81..a5380b9bab 100644
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -344,7 +344,7 @@ struct domain *alloc_domain_struct(void)
>>>     void free_domain_struct(struct domain *d)
>>>   {
>>> -    free_xenheap_page(d);
>>> +    free_xenheap_pages(d, get_order_from_bytes(sizeof(*d)));
>>
>> :(
>>
>> I'm entirely certain I raised this during the review of the original
>> patch.
>>
>> I'd much rather see the original patch reverted.  The current size of
>> struct domain with lockprofile enabled is 3200 bytes.
>
> Let me have a look first to see when/why struct domain is less than 4K
> with lockprofile.

In the intervening time, Juergen has totally rewritten lock profiling,
and the virtual timer infrastructure has been taken out-of-line.

Its probably the latter which is the dominating factor.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel