Re: [PATCH 03/17] x86: split __copy_{from,to}_user() into "guest" and "unsafe" variants

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Jan 14, 2021 at 04:04:32PM +0100, Jan Beulich wrote:
> The "guest" variants are intended to work with (potentially) fully guest
> controlled addresses, while the "unsafe" variants are not. Subsequently
> we will want them to have different behavior, so as first step identify
> which one is which. For now, both groups of constructs alias one another.
> 
> Double underscore prefixes are retained only on
> __copy_{from,to}_guest_pv(), to allow still distinguishing them from
> their "checking" counterparts once they also get renamed (to
> copy_{from,to}_guest_pv()).
> 
> Add previously missing __user at some call sites.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> Instead of __copy_{from,to}_guest_pv(), perhaps name them just
> __copy_{from,to}_pv()?
> 
> --- a/xen/arch/x86/gdbstub.c
> +++ b/xen/arch/x86/gdbstub.c
> @@ -33,13 +33,13 @@ gdb_arch_signal_num(struct cpu_user_regs
>  unsigned int
>  gdb_arch_copy_from_user(void *dest, const void *src, unsigned len)
>  {
> -    return __copy_from_user(dest, src, len);
> +    return copy_from_unsafe(dest, src, len);
>  }
>  
>  unsigned int 
>  gdb_arch_copy_to_user(void *dest, const void *src, unsigned len)
>  {
> -    return __copy_to_user(dest, src, len);
> +    return copy_to_unsafe(dest, src, len);

I assume we need to use the unsafe variants here, because the input
addresses are fully controlled by gdb, and hence not suitable as
speculation vectors?

Also could point to addresses belonging to both Xen or the guest
address space AFAICT.

> --- a/xen/include/asm-x86/uaccess.h
> +++ b/xen/include/asm-x86/uaccess.h

At some point we should also rename this to pvaccess.h maybe?

> @@ -197,21 +197,20 @@ do {
>  #define get_guest_size get_unsafe_size
>  
>  /**
> - * __copy_to_user: - Copy a block of data into user space, with less checking
> - * @to:   Destination address, in user space.
> - * @from: Source address, in kernel space.
> + * __copy_to_guest_pv: - Copy a block of data into guest space, with less
> + *                       checking

I would have preferred pv to be a prefix rather than a suffix, but we
already have the hvm accessors using that nomenclature.

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks, Roger.