[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] linux-user: Use signed lengths in uaccess.c
From: |
Laurent Vivier |
Subject: |
Re: [PATCH] linux-user: Use signed lengths in uaccess.c |
Date: |
Mon, 15 Mar 2021 22:07:13 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 |
Le 15/03/2021 à 21:40, Richard Henderson a écrit :
> Partially revert 09f679b62dff, but only for the length arguments.
> Instead of reverting to long, use ssize_t. Reinstate the > 0 check
> in unlock_user.
>
> Fixes: 09f679b62dff
> Reported-by: Coverity (CID 1446711)
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/qemu.h | 15 +++++++++------
> linux-user/uaccess.c | 12 ++++++------
> 2 files changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index 52c981710b..74e06e7121 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -627,8 +627,8 @@ static inline bool access_ok(CPUState *cpu, int type,
> * buffers between the target and host. These internally perform
> * locking/unlocking of the memory.
> */
> -int copy_from_user(void *hptr, abi_ulong gaddr, size_t len);
> -int copy_to_user(abi_ulong gaddr, void *hptr, size_t len);
> +int copy_from_user(void *hptr, abi_ulong gaddr, ssize_t len);
> +int copy_to_user(abi_ulong gaddr, void *hptr, ssize_t len);
>
> /* Functions for accessing guest memory. The tget and tput functions
> read/write single values, byteswapping as necessary. The lock_user
> function
> @@ -638,16 +638,19 @@ int copy_to_user(abi_ulong gaddr, void *hptr, size_t
> len);
>
> /* Lock an area of guest memory into the host. If copy is true then the
> host area will have the same contents as the guest. */
> -void *lock_user(int type, abi_ulong guest_addr, size_t len, bool copy);
> +void *lock_user(int type, abi_ulong guest_addr, ssize_t len, bool copy);
>
> /* Unlock an area of guest memory. The first LEN bytes must be
> flushed back to guest memory. host_ptr = NULL is explicitly
> allowed and does nothing. */
> #ifndef DEBUG_REMAP
> -static inline void unlock_user(void *host_ptr, abi_ulong guest_addr, size_t
> len)
> -{ }
> +static inline void unlock_user(void *host_ptr, abi_ulong guest_addr,
> + ssize_t len)
> +{
> + /* no-op */
> +}
> #else
> -void unlock_user(void *host_ptr, abi_ulong guest_addr, long len);
> +void unlock_user(void *host_ptr, abi_ulong guest_addr, ssize_t len);
> #endif
>
> /* Return the length of a string in target memory or -TARGET_EFAULT if
> diff --git a/linux-user/uaccess.c b/linux-user/uaccess.c
> index c696913016..82b833b8f1 100644
> --- a/linux-user/uaccess.c
> +++ b/linux-user/uaccess.c
> @@ -4,7 +4,7 @@
>
> #include "qemu.h"
>
> -void *lock_user(int type, abi_ulong guest_addr, size_t len, bool copy)
> +void *lock_user(int type, abi_ulong guest_addr, ssize_t len, bool copy)
> {
> void *host_addr;
>
> @@ -24,7 +24,7 @@ void *lock_user(int type, abi_ulong guest_addr, size_t len,
> bool copy)
> }
>
> #ifdef DEBUG_REMAP
> -void unlock_user(void *host_ptr, abi_ulong guest_addr, size_t len);
> +void unlock_user(void *host_ptr, abi_ulong guest_addr, ssize_t len);
The semicolon has been added by 687ca797893c ("linux-user: Move lock_user et al
out of line")
perhaps it's time to remove it?
Reviewed-by: Laurent Vivier <laurent@vivier.eu>