Re: [PATCH v5 19/31] linux-user: Handle tags in lock_user/unlock_user

[Peter Maydell]

On Wed, 3 Feb 2021 at 19:00, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Resolve the untagged address once, using thread_cpu.
> Tidy the DEBUG_REMAP code using glib routines.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/uaccess.c | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/linux-user/uaccess.c b/linux-user/uaccess.c
> index 30d01f8b30..c696913016 100644
> --- a/linux-user/uaccess.c
> +++ b/linux-user/uaccess.c
> @@ -6,36 +6,37 @@
>
>  void *lock_user(int type, abi_ulong guest_addr, size_t len, bool copy)
>  {
> +    void *host_addr;
> +
> +    guest_addr = cpu_untagged_addr(thread_cpu, guest_addr);
>      if (!access_ok_untagged(type, guest_addr, len)) {
>          return NULL;
>      }
> +    host_addr = g2h_untagged(guest_addr);
>  #ifdef DEBUG_REMAP
> -    {
> -        void *addr;
> -        addr = g_malloc(len);
> -        if (copy) {
> -            memcpy(addr, g2h(guest_addr), len);
> -        } else {
> -            memset(addr, 0, len);
> -        }
> -        return addr;
> +    if (copy) {
> +        host_addr = g_memdup(host_addr, len);
> +    } else {
> +        host_addr = g_malloc0(len);
>      }
> -#else
> -    return g2h_untagged(guest_addr);
>  #endif
> +    return host_addr;
>  }
>
>  #ifdef DEBUG_REMAP
>  void unlock_user(void *host_ptr, abi_ulong guest_addr, size_t len);
>  {
> +    void *host_ptr_conv;
> +
>      if (!host_ptr) {
>          return;
>      }
> -    if (host_ptr == g2h_untagged(guest_addr)) {
> +    host_ptr_conv = g2h(thread_cpu, guest_addr);
> +    if (host_ptr == host_ptr_conv) {
>          return;
>      }
> -    if (len > 0) {
> -        memcpy(g2h_untagged(guest_addr), host_ptr, len);
> +    if (len != 0) {
> +        memcpy(host_ptr_conv, host_ptr, len);
>      }

Why the change from checking >0 to checking !=0 ? I'd rather not
have to go through and audit all the callsites to confirm none
of them pass a "length-or-negative-errno" value here...

thanks
-- PMM