* [Qemu-devel] [PATCH] linux-user: Add missing check for return value of lock_user
@ 2015-03-14 15:12 ` Stefan Weil
0 siblings, 0 replies; 6+ messages in thread
From: Stefan Weil @ 2015-03-14 15:12 UTC (permalink / raw)
To: QEMU Trivial; +Cc: Stefan Weil, Riku Voipio, QEMU Developer
This fixes a warning from Coverity:
"Dereference null return value (NULL_RETURNS)"
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
linux-user/flatload.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/linux-user/flatload.c b/linux-user/flatload.c
index 566a7a8..56ac790 100644
--- a/linux-user/flatload.c
+++ b/linux-user/flatload.c
@@ -97,11 +97,13 @@ static int target_pread(int fd, abi_ulong ptr, abi_ulong len,
abi_ulong offset)
{
void *buf;
- int ret;
+ int ret = -TARGET_EFAULT;
buf = lock_user(VERIFY_WRITE, ptr, len, 0);
- ret = pread(fd, buf, len, offset);
- unlock_user(buf, ptr, len);
+ if (buf) {
+ ret = pread(fd, buf, len, offset);
+ unlock_user(buf, ptr, len);
+ }
return ret;
}
/****************************************************************************/
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH] linux-user: Add missing check for return value of lock_user
2015-03-14 15:12 ` [Qemu-devel] " Stefan Weil
@ 2015-03-15 11:15 ` Peter Maydell
-1 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2015-03-15 11:15 UTC (permalink / raw)
To: Stefan Weil; +Cc: QEMU Trivial, Riku Voipio, QEMU Developer
On 14 March 2015 at 15:12, Stefan Weil <sw@weilnetz.de> wrote:
> This fixes a warning from Coverity:
> "Dereference null return value (NULL_RETURNS)"
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> linux-user/flatload.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/linux-user/flatload.c b/linux-user/flatload.c
> index 566a7a8..56ac790 100644
> --- a/linux-user/flatload.c
> +++ b/linux-user/flatload.c
> @@ -97,11 +97,13 @@ static int target_pread(int fd, abi_ulong ptr, abi_ulong len,
> abi_ulong offset)
> {
> void *buf;
> - int ret;
> + int ret = -TARGET_EFAULT;
The return value for this function should be a host errno,
not a target errno, I think. (If you track back upwards
then eventually main.c calls loader_exec() and treats the
return value as a host errno.) This seems like the right
thing given that these loader functions are all pre-run
setup, and are not involved in emulation of guest syscalls.
>
> buf = lock_user(VERIFY_WRITE, ptr, len, 0);
> - ret = pread(fd, buf, len, offset);
> - unlock_user(buf, ptr, len);
> + if (buf) {
> + ret = pread(fd, buf, len, offset);
A different bug, but if ret here indicates that pread()
failed we should be returning -errno.
> + unlock_user(buf, ptr, len);
> + }
> return ret;
> }
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] linux-user: Add missing check for return value of lock_user
@ 2015-03-15 11:15 ` Peter Maydell
0 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2015-03-15 11:15 UTC (permalink / raw)
To: Stefan Weil; +Cc: QEMU Trivial, Riku Voipio, QEMU Developer
On 14 March 2015 at 15:12, Stefan Weil <sw@weilnetz.de> wrote:
> This fixes a warning from Coverity:
> "Dereference null return value (NULL_RETURNS)"
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> linux-user/flatload.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/linux-user/flatload.c b/linux-user/flatload.c
> index 566a7a8..56ac790 100644
> --- a/linux-user/flatload.c
> +++ b/linux-user/flatload.c
> @@ -97,11 +97,13 @@ static int target_pread(int fd, abi_ulong ptr, abi_ulong len,
> abi_ulong offset)
> {
> void *buf;
> - int ret;
> + int ret = -TARGET_EFAULT;
The return value for this function should be a host errno,
not a target errno, I think. (If you track back upwards
then eventually main.c calls loader_exec() and treats the
return value as a host errno.) This seems like the right
thing given that these loader functions are all pre-run
setup, and are not involved in emulation of guest syscalls.
>
> buf = lock_user(VERIFY_WRITE, ptr, len, 0);
> - ret = pread(fd, buf, len, offset);
> - unlock_user(buf, ptr, len);
> + if (buf) {
> + ret = pread(fd, buf, len, offset);
A different bug, but if ret here indicates that pread()
failed we should be returning -errno.
> + unlock_user(buf, ptr, len);
> + }
> return ret;
> }
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-trivial] [PATCH] linux-user: Add missing check for return value of lock_user
2015-03-14 15:12 ` [Qemu-devel] " Stefan Weil
@ 2015-10-19 14:28 ` Paolo Bonzini
-1 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2015-10-19 14:28 UTC (permalink / raw)
To: Stefan Weil, QEMU Trivial; +Cc: Riku Voipio, QEMU Developer
On 14/03/2015 16:12, Stefan Weil wrote:
> This fixes a warning from Coverity:
> "Dereference null return value (NULL_RETURNS)"
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> linux-user/flatload.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/linux-user/flatload.c b/linux-user/flatload.c
> index 566a7a8..56ac790 100644
> --- a/linux-user/flatload.c
> +++ b/linux-user/flatload.c
> @@ -97,11 +97,13 @@ static int target_pread(int fd, abi_ulong ptr, abi_ulong len,
> abi_ulong offset)
> {
> void *buf;
> - int ret;
> + int ret = -TARGET_EFAULT;
>
> buf = lock_user(VERIFY_WRITE, ptr, len, 0);
> - ret = pread(fd, buf, len, offset);
> - unlock_user(buf, ptr, len);
> + if (buf) {
> + ret = pread(fd, buf, len, offset);
> + unlock_user(buf, ptr, len);
> + }
> return ret;
> }
> /****************************************************************************/
>
Hi Stefan,
are you going to update and resend this patch?
Thanks,
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] linux-user: Add missing check for return value of lock_user
@ 2015-10-19 14:28 ` Paolo Bonzini
0 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2015-10-19 14:28 UTC (permalink / raw)
To: Stefan Weil, QEMU Trivial; +Cc: Riku Voipio, QEMU Developer
On 14/03/2015 16:12, Stefan Weil wrote:
> This fixes a warning from Coverity:
> "Dereference null return value (NULL_RETURNS)"
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> linux-user/flatload.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/linux-user/flatload.c b/linux-user/flatload.c
> index 566a7a8..56ac790 100644
> --- a/linux-user/flatload.c
> +++ b/linux-user/flatload.c
> @@ -97,11 +97,13 @@ static int target_pread(int fd, abi_ulong ptr, abi_ulong len,
> abi_ulong offset)
> {
> void *buf;
> - int ret;
> + int ret = -TARGET_EFAULT;
>
> buf = lock_user(VERIFY_WRITE, ptr, len, 0);
> - ret = pread(fd, buf, len, offset);
> - unlock_user(buf, ptr, len);
> + if (buf) {
> + ret = pread(fd, buf, len, offset);
> + unlock_user(buf, ptr, len);
> + }
> return ret;
> }
> /****************************************************************************/
>
Hi Stefan,
are you going to update and resend this patch?
Thanks,
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread