Re: [PATCH] x86/traps: fix an off-by-one error

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.05.2020 13:06, Hongyan Xia wrote:
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -300,6 +300,7 @@ static void show_guest_stack(struct vcpu *v, const struct 
> cpu_user_regs *regs)
>      int i;
>      unsigned long *stack, addr;
>      unsigned long mask = STACK_SIZE;
> +    void *stack_page = NULL;
>  
>      /* Avoid HVM as we don't know what the stack looks like. */
>      if ( is_hvm_vcpu(v) )
> @@ -328,7 +329,7 @@ static void show_guest_stack(struct vcpu *v, const struct 
> cpu_user_regs *regs)
>          vcpu = maddr_get_owner(read_cr3()) == v->domain ? v : NULL;
>          if ( !vcpu )
>          {
> -            stack = do_page_walk(v, (unsigned long)stack);
> +            stack_page = stack = do_page_walk(v, (unsigned long)stack);
>              if ( (unsigned long)stack < PAGE_SIZE )
>              {
>                  printk("Inaccessible guest memory.\n");
> @@ -358,7 +359,7 @@ static void show_guest_stack(struct vcpu *v, const struct 
> cpu_user_regs *regs)
>      if ( mask == PAGE_SIZE )
>      {
>          BUILD_BUG_ON(PAGE_SIZE == STACK_SIZE);
> -        unmap_domain_page(stack);
> +        unmap_domain_page(stack_page);
>      }

With this I think you want to change the whole construct here to

    if ( stack_page )
        unmap_domain_page(stack_page);

i.e. with the then no longer relevant BUILD_BUG_ON() also dropped.

What's more important though - please also fix the same issue in
compat_show_guest_stack(). Unless I'm mistaken of course, in which
case it would be nice if the description could mention why the
other similar function isn't affected.

Jan