[PATCH] linux-user: Check copy_from_user() return value in vma_dump_size()

[PATCH] linux-user: Check copy_from_user() return value in vma_dump_size()

[Peter Maydell]

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>
---
 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 bf8c1bd2533..e19d0b5cb05 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -3484,7 +3484,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.20.1

Re: [PATCH] linux-user: Check copy_from_user() return value in vma_dump_size()

[no-reply]

Patchew URL: https://patchew.org/QEMU/20201103141532.19912-1-peter.maydell@linaro.org/



Hi,

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

Type: series
Message-id: 20201103141532.19912-1-peter.maydell@linaro.org
Subject: [PATCH] linux-user: Check copy_from_user() return value in vma_dump_size()

=== 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
Switched to a new branch 'test'
421265c linux-user: Check copy_from_user() return value in vma_dump_size()

=== OUTPUT BEGIN ===
ERROR: space prohibited between function name and open parenthesis '('
#26: FILE: linux-user/elfload.c:3487:
+        if (copy_from_user(page, vma->vma_start, sizeof (page))) {

total: 1 errors, 0 warnings, 10 lines checked

Commit 421265c84dcb (linux-user: Check copy_from_user() return value in vma_dump_size()) 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/20201103141532.19912-1-peter.maydell@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH] linux-user: Check copy_from_user() return value in vma_dump_size()

[Laurent Vivier]

Le 03/11/2020 à 15:15, Peter Maydell a écrit :
> 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>
> ---
>  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 bf8c1bd2533..e19d0b5cb05 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -3484,7 +3484,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) &&
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH] linux-user: Check copy_from_user() return value in vma_dump_size()

[Laurent Vivier]

Le 03/11/2020 à 15:15, Peter Maydell a écrit :
> 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>
> ---
>  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 bf8c1bd2533..e19d0b5cb05 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -3484,7 +3484,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) &&
> 

Applied to my linux-user-for-5.2 branch.

Thanks,
Laurent