Re: [PATCH] x86/smpboot: Unconditionally call memguard_unguard_stack() in cpu_smpboot_free()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.10.2020 15:23, Andrew Cooper wrote:
> On 13/10/2020 14:16, Jan Beulich wrote:
>> On 05.10.2020 14:23, Andrew Cooper wrote:
>>> --- a/xen/arch/x86/smpboot.c
>>> +++ b/xen/arch/x86/smpboot.c
>>> @@ -971,16 +971,16 @@ static void cpu_smpboot_free(unsigned int cpu, bool 
>>> remove)
>>>      if ( IS_ENABLED(CONFIG_PV32) )
>>>          FREE_XENHEAP_PAGE(per_cpu(compat_gdt, cpu));
>>>  
>>> +    if ( stack_base[cpu] )
>>> +        memguard_unguard_stack(stack_base[cpu]);
>>> +
>>>      if ( remove )
>>>      {
>>>          FREE_XENHEAP_PAGE(per_cpu(gdt, cpu));
>>>          FREE_XENHEAP_PAGE(idt_tables[cpu]);
>>>  
>>>          if ( stack_base[cpu] )
>>> -        {
>>> -            memguard_unguard_stack(stack_base[cpu]);
>>>              FREE_XENHEAP_PAGES(stack_base[cpu], STACK_ORDER);
>>> -        }
>>>      }
>>>  }
>> In my initial reply to Marek's report I did suggest putting the fix
>> in the alloc path in order to keep the pages "guarded" while the CPU
>> is parked
> 
> In which case you should have identified that bug explicitly.
> 
> Because I can't read your mind while fixing the other real bugs in your
> suggestion.

I'm sorry for the brevity at that point - it was a Sunday, and I merely
thought I'd write down my observation after reading the report. And of
course I'm curious as to the other real bugs in my suggestion (when I
anyway said "something like").

Jan