On Wed, Sep 29, 2021 at 11:42:34AM +0200, Jan Beulich wrote:
> The conversion to __get_guest() failed to account for the fact that for
> remote vCPU-s dumping gets done through a pointer obtained from
> map_domain_page(): __get_guest() arranges for (apparent) accesses to
> hypervisor space to cause #GP(0).
>
> Fixes: 6a1d72d3739e ('x86: split __{get,put}_user() into "guest" and "unsafe"
> variants')
> Signed-off-by: Jan Beulich <[email protected]>
> ---
> Using get_unsafe() might be an option as well, instead of the added
> extra conditional.
A third option might be to place them in a non Xen private VA range,
like we do for the compat argument translation. That's however too
much IMO.
>
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -275,7 +275,9 @@ static void compat_show_guest_stack(stru
> {
> if ( (((long)stack - 1) ^ ((long)(stack + 1) - 1)) & mask )
> break;
> - if ( __get_guest(addr, stack) )
> + if ( stack_page )
> + addr = *stack;
> + else if ( __get_guest(addr, stack) )
> {
> if ( i != 0 )
> printk("\n ");
> @@ -344,7 +346,9 @@ static void show_guest_stack(struct vcpu
> {
> if ( (((long)stack - 1) ^ ((long)(stack + 1) - 1)) & mask )
> break;
> - if ( __get_guest(addr, stack) )
> + if ( stack_page )
> + addr = *stack;
I don't really have a strong opinion regarding this or the usage of
get_unsafe. When v == current we have the extra protection of being
sure no private Xen mappings are accessed, but that's only for a
single vCPU at most, at which point we might just use get_unsafe
uniformly. I guess I have a slight preference for get_unsafe because
using get_guest doesn't really get us anything.
If you keep it in the current form, or decide to switch to
get_unsafe:
Reviewed-by: Roger Pau Monné <[email protected]>
Thanks, Roger.
