qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL 0/4] Linux user for 5.2 patches
@ 2020-11-05  7:08 Laurent Vivier
  2020-11-05  7:08 ` [PULL 1/4] linux-user/mips/cpu_loop: silence the compiler warnings Laurent Vivier
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Laurent Vivier @ 2020-11-05  7:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier

The following changes since commit 8680d6e36468f1ca00e2fe749bef50585d632401:

  Merge remote-tracking branch 'remotes/nvme/tags/pull-nvme-20201102' into st=
aging (2020-11-02 17:17:29 +0000)

are available in the Git repository at:

  git://github.com/vivier/qemu.git tags/linux-user-for-5.2-pull-request

for you to fetch changes up to 022625a8ade3005addb42700a145bae6a1653240:

  linux-user: Check copy_from_user() return value in vma_dump_size() (2020-11=
-04 22:28:05 +0100)

----------------------------------------------------------------
Coverity and compiler warning fixes

----------------------------------------------------------------

Alistair Francis (1):
  linux-user/syscall: Fix missing target_to_host_timespec64() check

Chen Qun (1):
  linux-user/mips/cpu_loop: silence the compiler warnings

Peter Maydell (2):
  linux-user: Use "!=3D 0" when checking if MAP_FIXED_NOREPLACE is
    non-zero
  linux-user: Check copy_from_user() return value in vma_dump_size()

 linux-user/elfload.c       | 7 +++++--
 linux-user/mips/cpu_loop.c | 4 ++++
 linux-user/syscall.c       | 4 +++-
 3 files changed, 12 insertions(+), 3 deletions(-)

--=20
2.28.0



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PULL 1/4] linux-user/mips/cpu_loop: silence the compiler warnings
  2020-11-05  7:08 [PULL 0/4] Linux user for 5.2 patches Laurent Vivier
@ 2020-11-05  7:08 ` Laurent Vivier
  2020-11-05  7:08 ` [PULL 2/4] linux-user: Use "!= 0" when checking if MAP_FIXED_NOREPLACE is non-zero Laurent Vivier
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Laurent Vivier @ 2020-11-05  7:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: Chen Qun, Thomas Huth, Laurent Vivier, Euler Robot

From: Chen Qun <kuhn.chenqun@huawei.com>

When using -Wimplicit-fallthrough in our CFLAGS, the compiler showed warning:
linux-user/mips/cpu_loop.c: In function ‘cpu_loop’:
linux-user/mips/cpu_loop.c:104:24: warning: this statement may fall through [-Wimplicit-fallthrough=]
  104 |                     if ((ret = get_user_ual(arg8, sp_reg + 28)) != 0) {
      |                        ^
linux-user/mips/cpu_loop.c:107:17: note: here
  107 |                 case 7:
      |                 ^~~~
linux-user/mips/cpu_loop.c:108:24: warning: this statement may fall through [-Wimplicit-fallthrough=]
  108 |                     if ((ret = get_user_ual(arg7, sp_reg + 24)) != 0) {
      |                        ^
linux-user/mips/cpu_loop.c:111:17: note: here
  111 |                 case 6:
      |                 ^~~~
linux-user/mips/cpu_loop.c:112:24: warning: this statement may fall through [-Wimplicit-fallthrough=]
  112 |                     if ((ret = get_user_ual(arg6, sp_reg + 20)) != 0) {
      |                        ^
linux-user/mips/cpu_loop.c:115:17: note: here
  115 |                 case 5:
      |                 ^~~~

Add the corresponding "fall through" comment to fix it.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20201030004046.2191790-5-kuhn.chenqun@huawei.com>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/mips/cpu_loop.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/linux-user/mips/cpu_loop.c b/linux-user/mips/cpu_loop.c
index 553e8ca7f576..cfe7ba5c47d8 100644
--- a/linux-user/mips/cpu_loop.c
+++ b/linux-user/mips/cpu_loop.c
@@ -104,18 +104,22 @@ void cpu_loop(CPUMIPSState *env)
                     if ((ret = get_user_ual(arg8, sp_reg + 28)) != 0) {
                         goto done_syscall;
                     }
+                    /* fall through */
                 case 7:
                     if ((ret = get_user_ual(arg7, sp_reg + 24)) != 0) {
                         goto done_syscall;
                     }
+                    /* fall through */
                 case 6:
                     if ((ret = get_user_ual(arg6, sp_reg + 20)) != 0) {
                         goto done_syscall;
                     }
+                    /* fall through */
                 case 5:
                     if ((ret = get_user_ual(arg5, sp_reg + 16)) != 0) {
                         goto done_syscall;
                     }
+                    /* fall through */
                 default:
                     break;
                 }
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PULL 2/4] linux-user: Use "!= 0" when checking if MAP_FIXED_NOREPLACE is non-zero
  2020-11-05  7:08 [PULL 0/4] Linux user for 5.2 patches Laurent Vivier
  2020-11-05  7:08 ` [PULL 1/4] linux-user/mips/cpu_loop: silence the compiler warnings Laurent Vivier
@ 2020-11-05  7:08 ` Laurent Vivier
  2020-11-05  7:08 ` [PULL 3/4] linux-user/syscall: Fix missing target_to_host_timespec64() check Laurent Vivier
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Laurent Vivier @ 2020-11-05  7:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Laurent Vivier, Philippe Mathieu-Daudé

From: Peter Maydell <peter.maydell@linaro.org>

In pgd_find_hole_fallback(), Coverity doesn't like the use
of "if (MAP_FIXED_NOREPLACE || ...)" because it's using a
logical operator on a constant other than 0 or 1 and its
heuristic thinks we might have intended a bitwise operator
instead.

The logic is correct (we are checking whether the host really
has a MAP_FIXED_NOREPLACE or whether we fell back to the
"#define as 0 to ignore" from osdep.h); make Coverity
happier by explicitly writing out the comparison with zero.

Fixes: Coverity CID 1431059
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20201103142636.21125-1-peter.maydell@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/elfload.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index bf8c1bd25330..cae41d504d36 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2188,7 +2188,8 @@ static uintptr_t pgd_find_hole_fallback(uintptr_t guest_size, uintptr_t brk,
                                      PROT_NONE, flags, -1, 0);
             if (mmap_start != MAP_FAILED) {
                 munmap((void *) align_start, guest_size);
-                if (MAP_FIXED_NOREPLACE || mmap_start == (void *) align_start) {
+                if (MAP_FIXED_NOREPLACE != 0 ||
+                    mmap_start == (void *) align_start) {
                     return (uintptr_t) mmap_start + offset;
                 }
             }
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PULL 3/4] linux-user/syscall: Fix missing target_to_host_timespec64() check
  2020-11-05  7:08 [PULL 0/4] Linux user for 5.2 patches Laurent Vivier
  2020-11-05  7:08 ` [PULL 1/4] linux-user/mips/cpu_loop: silence the compiler warnings Laurent Vivier
  2020-11-05  7:08 ` [PULL 2/4] linux-user: Use "!= 0" when checking if MAP_FIXED_NOREPLACE is non-zero Laurent Vivier
@ 2020-11-05  7:08 ` Laurent Vivier
  2020-11-05  7:08 ` [PULL 4/4] linux-user: Check copy_from_user() return value in vma_dump_size() Laurent Vivier
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Laurent Vivier @ 2020-11-05  7:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alistair Francis, Laurent Vivier, Philippe Mathieu-Daudé

From: Alistair Francis <alistair.francis@wdc.com>

Coverity pointed out (CID 1432339) that target_to_host_timespec64() can
fail with -TARGET_EFAULT but we never check the return value. This patch
checks the return value and handles the error.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <cad74fae734d2562746b94acd9c34b00081c89bf.1604432881.git.alistair.francis@wdc.com>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/syscall.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 6fef8181e738..3160a9ba06bd 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7592,7 +7592,9 @@ static int do_futex_time64(target_ulong uaddr, int op, int val, target_ulong tim
     case FUTEX_WAIT_BITSET:
         if (timeout) {
             pts = &ts;
-            target_to_host_timespec64(pts, timeout);
+            if (target_to_host_timespec64(pts, timeout)) {
+                return -TARGET_EFAULT;
+            }
         } else {
             pts = NULL;
         }
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PULL 4/4] linux-user: Check copy_from_user() return value in vma_dump_size()
  2020-11-05  7:08 [PULL 0/4] Linux user for 5.2 patches Laurent Vivier
                   ` (2 preceding siblings ...)
  2020-11-05  7:08 ` [PULL 3/4] linux-user/syscall: Fix missing target_to_host_timespec64() check Laurent Vivier
@ 2020-11-05  7:08 ` Laurent Vivier
  2020-11-05  7:15 ` [PULL 0/4] Linux user for 5.2 patches no-reply
  2020-11-06  9:40 ` Peter Maydell
  5 siblings, 0 replies; 7+ messages in thread
From: Laurent Vivier @ 2020-11-05  7:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Laurent Vivier

From: Peter Maydell <peter.maydell@linaro.org>

Coverity points out that we don't check the return value from
copy_from_user() in vma_dump_size(). This is to some extent
a "can't happen" error since we've already checked the page
with an access_ok() call earlier, but it's simple enough to
handle the error anyway.

Fixes: Coverity CID 1432362
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20201103141532.19912-1-peter.maydell@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/elfload.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index cae41d504d36..0b02a926025e 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -3485,7 +3485,9 @@ static abi_ulong vma_dump_size(const struct vm_area_struct *vma)
     if (vma->vma_flags & PROT_EXEC) {
         char page[TARGET_PAGE_SIZE];
 
-        copy_from_user(page, vma->vma_start, sizeof (page));
+        if (copy_from_user(page, vma->vma_start, sizeof (page))) {
+            return 0;
+        }
         if ((page[EI_MAG0] == ELFMAG0) &&
             (page[EI_MAG1] == ELFMAG1) &&
             (page[EI_MAG2] == ELFMAG2) &&
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PULL 0/4] Linux user for 5.2 patches
  2020-11-05  7:08 [PULL 0/4] Linux user for 5.2 patches Laurent Vivier
                   ` (3 preceding siblings ...)
  2020-11-05  7:08 ` [PULL 4/4] linux-user: Check copy_from_user() return value in vma_dump_size() Laurent Vivier
@ 2020-11-05  7:15 ` no-reply
  2020-11-06  9:40 ` Peter Maydell
  5 siblings, 0 replies; 7+ messages in thread
From: no-reply @ 2020-11-05  7:15 UTC (permalink / raw)
  To: laurent; +Cc: qemu-devel, laurent

Patchew URL: https://patchew.org/QEMU/20201105070837.558332-1-laurent@vivier.eu/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20201105070837.558332-1-laurent@vivier.eu
Subject: [PULL 0/4] Linux user for 5.2 patches

=== 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/20201105070837.558332-1-laurent@vivier.eu -> patchew/20201105070837.558332-1-laurent@vivier.eu
Switched to a new branch 'test'
11627c2 linux-user: Check copy_from_user() return value in vma_dump_size()
51fc3d6 linux-user/syscall: Fix missing target_to_host_timespec64() check
60ba650 linux-user: Use "!= 0" when checking if MAP_FIXED_NOREPLACE is non-zero
77abc4c linux-user/mips/cpu_loop: silence the compiler warnings

=== OUTPUT BEGIN ===
1/4 Checking commit 77abc4ceda78 (linux-user/mips/cpu_loop: silence the compiler warnings)
2/4 Checking commit 60ba6508037c (linux-user: Use "!= 0" when checking if MAP_FIXED_NOREPLACE is non-zero)
3/4 Checking commit 51fc3d6ec118 (linux-user/syscall: Fix missing target_to_host_timespec64() check)
4/4 Checking commit 11627c281598 (linux-user: Check copy_from_user() return value in vma_dump_size())
ERROR: space prohibited between function name and open parenthesis '('
#29: FILE: linux-user/elfload.c:3488:
+        if (copy_from_user(page, vma->vma_start, sizeof (page))) {

total: 1 errors, 0 warnings, 10 lines checked

Patch 4/4 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/20201105070837.558332-1-laurent@vivier.eu/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] 7+ messages in thread

* Re: [PULL 0/4] Linux user for 5.2 patches
  2020-11-05  7:08 [PULL 0/4] Linux user for 5.2 patches Laurent Vivier
                   ` (4 preceding siblings ...)
  2020-11-05  7:15 ` [PULL 0/4] Linux user for 5.2 patches no-reply
@ 2020-11-06  9:40 ` Peter Maydell
  5 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2020-11-06  9:40 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: QEMU Developers

On Thu, 5 Nov 2020 at 07:10, Laurent Vivier <laurent@vivier.eu> wrote:
>
> The following changes since commit 8680d6e36468f1ca00e2fe749bef50585d632401:
>
>   Merge remote-tracking branch 'remotes/nvme/tags/pull-nvme-20201102' into st=
> aging (2020-11-02 17:17:29 +0000)
>
> are available in the Git repository at:
>
>   git://github.com/vivier/qemu.git tags/linux-user-for-5.2-pull-request
>
> for you to fetch changes up to 022625a8ade3005addb42700a145bae6a1653240:
>
>   linux-user: Check copy_from_user() return value in vma_dump_size() (2020-11=
> -04 22:28:05 +0100)
>
> ----------------------------------------------------------------
> Coverity and compiler warning fixes
>
> ----------------------------------------------------------------


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-11-06  9:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-05  7:08 [PULL 0/4] Linux user for 5.2 patches Laurent Vivier
2020-11-05  7:08 ` [PULL 1/4] linux-user/mips/cpu_loop: silence the compiler warnings Laurent Vivier
2020-11-05  7:08 ` [PULL 2/4] linux-user: Use "!= 0" when checking if MAP_FIXED_NOREPLACE is non-zero Laurent Vivier
2020-11-05  7:08 ` [PULL 3/4] linux-user/syscall: Fix missing target_to_host_timespec64() check Laurent Vivier
2020-11-05  7:08 ` [PULL 4/4] linux-user: Check copy_from_user() return value in vma_dump_size() Laurent Vivier
2020-11-05  7:15 ` [PULL 0/4] Linux user for 5.2 patches no-reply
2020-11-06  9:40 ` Peter Maydell

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).