* [PATCH] linux-user: Use signed lengths in uaccess.c
@ 2021-03-15 20:40 Richard Henderson
2021-03-15 20:45 ` no-reply
2021-03-15 21:07 ` Laurent Vivier
0 siblings, 2 replies; 5+ messages in thread
From: Richard Henderson @ 2021-03-15 20:40 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, laurent
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);
{
void *host_ptr_conv;
@@ -35,7 +35,7 @@ void unlock_user(void *host_ptr, abi_ulong guest_addr, size_t len);
if (host_ptr == host_ptr_conv) {
return;
}
- if (len != 0) {
+ if (len > 0) {
memcpy(host_ptr_conv, host_ptr, len);
}
g_free(host_ptr);
@@ -48,14 +48,14 @@ void *lock_user_string(abi_ulong guest_addr)
if (len < 0) {
return NULL;
}
- return lock_user(VERIFY_READ, guest_addr, (size_t)len + 1, 1);
+ return lock_user(VERIFY_READ, guest_addr, len + 1, 1);
}
/* copy_from_user() and copy_to_user() are usually used to copy data
* 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_from_user(void *hptr, abi_ulong gaddr, ssize_t len)
{
int ret = 0;
void *ghptr = lock_user(VERIFY_READ, gaddr, len, 1);
@@ -69,7 +69,7 @@ int copy_from_user(void *hptr, abi_ulong gaddr, size_t len)
return ret;
}
-int copy_to_user(abi_ulong gaddr, void *hptr, size_t len)
+int copy_to_user(abi_ulong gaddr, void *hptr, ssize_t len)
{
int ret = 0;
void *ghptr = lock_user(VERIFY_WRITE, gaddr, len, 0);
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] linux-user: Use signed lengths in uaccess.c
2021-03-15 20:40 [PATCH] linux-user: Use signed lengths in uaccess.c Richard Henderson
@ 2021-03-15 20:45 ` no-reply
2021-03-15 21:07 ` Laurent Vivier
1 sibling, 0 replies; 5+ messages in thread
From: no-reply @ 2021-03-15 20:45 UTC (permalink / raw)
To: richard.henderson; +Cc: peter.maydell, qemu-devel, laurent
Patchew URL: https://patchew.org/QEMU/20210315204004.2025219-1-richard.henderson@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210315204004.2025219-1-richard.henderson@linaro.org
Subject: [PATCH] linux-user: Use signed lengths in uaccess.c
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/20210315204004.2025219-1-richard.henderson@linaro.org -> patchew/20210315204004.2025219-1-richard.henderson@linaro.org
Switched to a new branch 'test'
cabd5c4 linux-user: Use signed lengths in uaccess.c
=== OUTPUT BEGIN ===
ERROR: externs should be avoided in .c files
#77: FILE: linux-user/uaccess.c:27:
+void unlock_user(void *host_ptr, abi_ulong guest_addr, ssize_t len);
total: 1 errors, 0 warnings, 81 lines checked
Commit cabd5c49328f (linux-user: Use signed lengths in uaccess.c) has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20210315204004.2025219-1-richard.henderson@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] linux-user: Use signed lengths in uaccess.c
2021-03-15 20:40 [PATCH] linux-user: Use signed lengths in uaccess.c Richard Henderson
2021-03-15 20:45 ` no-reply
@ 2021-03-15 21:07 ` Laurent Vivier
2021-04-07 15:16 ` Peter Maydell
1 sibling, 1 reply; 5+ messages in thread
From: Laurent Vivier @ 2021-03-15 21:07 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: peter.maydell
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>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] linux-user: Use signed lengths in uaccess.c
2021-03-15 21:07 ` Laurent Vivier
@ 2021-04-07 15:16 ` Peter Maydell
2021-04-07 16:49 ` Laurent Vivier
0 siblings, 1 reply; 5+ messages in thread
From: Peter Maydell @ 2021-04-07 15:16 UTC (permalink / raw)
To: Laurent Vivier; +Cc: Richard Henderson, QEMU Developers
On Mon, 15 Mar 2021 at 21:07, Laurent Vivier <laurent@vivier.eu> wrote:
>
> 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>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Hi -- did this patch get lost? I think we should put it into 6.0.
thanks
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] linux-user: Use signed lengths in uaccess.c
2021-04-07 15:16 ` Peter Maydell
@ 2021-04-07 16:49 ` Laurent Vivier
0 siblings, 0 replies; 5+ messages in thread
From: Laurent Vivier @ 2021-04-07 16:49 UTC (permalink / raw)
To: Peter Maydell; +Cc: Richard Henderson, QEMU Developers
Le 07/04/2021 à 17:16, Peter Maydell a écrit :
> On Mon, 15 Mar 2021 at 21:07, Laurent Vivier <laurent@vivier.eu> wrote:
>>
>> 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>
>
>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>
> Hi -- did this patch get lost? I think we should put it into 6.0.
It seems so... thank you.
I prepare a PR with it.
Laurent
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-04-07 16:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-15 20:40 [PATCH] linux-user: Use signed lengths in uaccess.c Richard Henderson
2021-03-15 20:45 ` no-reply
2021-03-15 21:07 ` Laurent Vivier
2021-04-07 15:16 ` Peter Maydell
2021-04-07 16:49 ` Laurent Vivier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).