1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH] syscall: fixed mincore(2) not failing with ENOMEM

Franklin Snaipe Mathieu posted 1 patch 7 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170216085857.23501-1-snaipe@diacritic.io
Test checkpatch passed
Test docker passed
Test s390x failed
There is a newer version of this series
linux-user/syscall.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
[Qemu-devel] [PATCH] syscall: fixed mincore(2) not failing with ENOMEM
Posted by Franklin Snaipe Mathieu 7 years, 2 months ago 
From: "Franklin \"Snaipe\" Mathieu" <snaipe@diacritic.io>

The current implementation of the mincore(2) syscall sets errno to
EFAULT when the region identified by the first two parameters is
invalid.

This goes against the man page specification, where mincore(2) should
only fail with EFAULT when the third parameter is an invalid address;
and fail with ENOMEM when the checked region does not point to mapped
memory.

Signed-off-by: Franklin "Snaipe" Mathieu <snaipe@diacritic.io>
Cc: Riku Voipio <riku.voipio@linaro.org>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
 linux-user/syscall.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9be8e9530e..af0dd40631 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -11061,11 +11061,16 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     case TARGET_NR_mincore:
         {
             void *a;
+            ret = -TARGET_ENOMEM;
+            a = lock_user(VERIFY_READ, arg1, arg2, 0);
+            if (!a) {
+                goto fail;
+            }
             ret = -TARGET_EFAULT;
-            if (!(a = lock_user(VERIFY_READ, arg1,arg2, 0)))
-                goto efault;
-            if (!(p = lock_user_string(arg3)))
+            p = lock_user_string(arg3)
+            if (!p) {
                 goto mincore_fail;
+            }
             ret = get_errno(mincore(a, arg2, p));
             unlock_user(p, arg3, ret);
             mincore_fail:
-- 
2.11.0
Re: [Qemu-devel] [PATCH] syscall: fixed mincore(2) not failing with ENOMEM
Posted by Fam Zheng 7 years, 2 months ago 
On Thu, 02/16 08:58, Franklin Snaipe Mathieu wrote:
> +            p = lock_user_string(arg3)
> +            if (!p) {

Please compile test at least, even if it is a trivial patch.

Fam
Re: [Qemu-devel] [PATCH] syscall: fixed mincore(2) not failing with ENOMEM
Posted by Franklin "Snaipe" Mathieu 7 years, 2 months ago 
2017-02-17 0:50 GMT+00:00 Fam Zheng <famz@redhat.com>:
> On Thu, 02/16 08:58, Franklin Snaipe Mathieu wrote:
>> +            p = lock_user_string(arg3)
>> +            if (!p) {
>
> Please compile test at least, even if it is a trivial patch.
>

Whoops, sorry, I forgot to re-compile after fixing the checkpatch issues.

-- 
Franklin "Snaipe" Mathieu
🝰 https://diacritic.io
Re: [Qemu-devel] [PATCH] syscall: fixed mincore(2) not failing with ENOMEM
Posted by Michael Tokarev 7 years, 1 month ago 
16.02.2017 11:58, Franklin Snaipe Mathieu wrote:
> From: "Franklin \"Snaipe\" Mathieu" <snaipe@diacritic.io>
> 
> The current implementation of the mincore(2) syscall sets errno to
> EFAULT when the region identified by the first two parameters is
> invalid.
> 
> This goes against the man page specification, where mincore(2) should
> only fail with EFAULT when the third parameter is an invalid address;
> and fail with ENOMEM when the checked region does not point to mapped
> memory.

Applied to -trivial, thanks!

/mjt
[Qemu-devel] [PATCH] syscall: fixed mincore(2) not failing with ENOMEM
Posted by Franklin Snaipe Mathieu 7 years, 2 months ago 
From: "Franklin \"Snaipe\" Mathieu" <snaipe@diacritic.io>

The current implementation of the mincore(2) syscall sets errno to
EFAULT when the region identified by the first two parameters is
invalid.

This goes against the man page specification, where mincore(2) should
only fail with EFAULT when the third parameter is an invalid address;
and fail with ENOMEM when the checked region does not point to mapped
memory.

Signed-off-by: Franklin "Snaipe" Mathieu <snaipe@diacritic.io>
Cc: Riku Voipio <riku.voipio@linaro.org>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
 linux-user/syscall.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9be8e9530e..733e0009e1 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -11061,11 +11061,16 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     case TARGET_NR_mincore:
         {
             void *a;
+            ret = -TARGET_ENOMEM;
+            a = lock_user(VERIFY_READ, arg1, arg2, 0);
+            if (!a) {
+                goto fail;
+            }
             ret = -TARGET_EFAULT;
-            if (!(a = lock_user(VERIFY_READ, arg1,arg2, 0)))
-                goto efault;
-            if (!(p = lock_user_string(arg3)))
+            p = lock_user_string(arg3);
+            if (!p) {
                 goto mincore_fail;
+            }
             ret = get_errno(mincore(a, arg2, p));
             unlock_user(p, arg3, ret);
             mincore_fail:
-- 
2.11.0
Re: [Qemu-devel] [PATCH] syscall: fixed mincore(2) not failing with ENOMEM
Posted by Franklin "Snaipe" Mathieu 7 years, 2 months ago 
Ignore this patch, this was sent again by mistake.

2017-02-17 8:57 GMT+00:00 Franklin Snaipe Mathieu <snaipe@diacritic.io>:
> From: "Franklin \"Snaipe\" Mathieu" <snaipe@diacritic.io>
>
> The current implementation of the mincore(2) syscall sets errno to
> EFAULT when the region identified by the first two parameters is
> invalid.
>
> This goes against the man page specification, where mincore(2) should
> only fail with EFAULT when the third parameter is an invalid address;
> and fail with ENOMEM when the checked region does not point to mapped
> memory.
>
> Signed-off-by: Franklin "Snaipe" Mathieu <snaipe@diacritic.io>
> Cc: Riku Voipio <riku.voipio@linaro.org>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  linux-user/syscall.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 9be8e9530e..733e0009e1 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -11061,11 +11061,16 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>      case TARGET_NR_mincore:
>          {
>              void *a;
> +            ret = -TARGET_ENOMEM;
> +            a = lock_user(VERIFY_READ, arg1, arg2, 0);
> +            if (!a) {
> +                goto fail;
> +            }
>              ret = -TARGET_EFAULT;
> -            if (!(a = lock_user(VERIFY_READ, arg1,arg2, 0)))
> -                goto efault;
> -            if (!(p = lock_user_string(arg3)))
> +            p = lock_user_string(arg3);
> +            if (!p) {
>                  goto mincore_fail;
> +            }
>              ret = get_errno(mincore(a, arg2, p));
>              unlock_user(p, arg3, ret);
>              mincore_fail:
> --
> 2.11.0
>



-- 
Franklin "Snaipe" Mathieu
🝰 https://diacritic.io
Re: [Qemu-devel] [PATCH] syscall: fixed mincore(2) not failing with ENOMEM
Posted by Fam Zheng 7 years, 2 months ago 
On Fri, 02/17 09:00, Franklin "Snaipe" Mathieu wrote:
> Ignore this patch, this was sent again by mistake.

Hi Franklin, thanks for the re-submission. Next time please send consecutive
versions as top-level threads instead of replies to previous versions, with
proper 'v2", "v3" tags in the subject.

Fam
[Qemu-devel] [PATCH v2] syscall: fixed mincore(2) not failing with ENOMEM
Posted by Franklin Snaipe Mathieu 7 years, 2 months ago 
From: "Franklin \"Snaipe\" Mathieu" <snaipe@diacritic.io>

The current implementation of the mincore(2) syscall sets errno to
EFAULT when the region identified by the first two parameters is
invalid.

This goes against the man page specification, where mincore(2) should
only fail with EFAULT when the third parameter is an invalid address;
and fail with ENOMEM when the checked region does not point to mapped
memory.

Signed-off-by: Franklin "Snaipe" Mathieu <snaipe@diacritic.io>
Cc: Riku Voipio <riku.voipio@linaro.org>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
 linux-user/syscall.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9be8e9530e..733e0009e1 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -11061,11 +11061,16 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     case TARGET_NR_mincore:
         {
             void *a;
+            ret = -TARGET_ENOMEM;
+            a = lock_user(VERIFY_READ, arg1, arg2, 0);
+            if (!a) {
+                goto fail;
+            }
             ret = -TARGET_EFAULT;
-            if (!(a = lock_user(VERIFY_READ, arg1,arg2, 0)))
-                goto efault;
-            if (!(p = lock_user_string(arg3)))
+            p = lock_user_string(arg3);
+            if (!p) {
                 goto mincore_fail;
+            }
             ret = get_errno(mincore(a, arg2, p));
             unlock_user(p, arg3, ret);
             mincore_fail:
-- 
2.11.0
Re: [Qemu-devel] [PATCH v2] syscall: fixed mincore(2) not failing with ENOMEM
Posted by Laurent Vivier 7 years, 1 month ago 
Le 17/02/2017 à 09:58, Franklin Snaipe Mathieu a écrit :
> From: "Franklin \"Snaipe\" Mathieu" <snaipe@diacritic.io>
> 
> The current implementation of the mincore(2) syscall sets errno to
> EFAULT when the region identified by the first two parameters is
> invalid.
> 
> This goes against the man page specification, where mincore(2) should
> only fail with EFAULT when the third parameter is an invalid address;
> and fail with ENOMEM when the checked region does not point to mapped
> memory.
> 
> Signed-off-by: Franklin "Snaipe" Mathieu <snaipe@diacritic.io>
> Cc: Riku Voipio <riku.voipio@linaro.org>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  linux-user/syscall.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 9be8e9530e..733e0009e1 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -11061,11 +11061,16 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>      case TARGET_NR_mincore:
>          {
>              void *a;
> +            ret = -TARGET_ENOMEM;
> +            a = lock_user(VERIFY_READ, arg1, arg2, 0);
> +            if (!a) {
> +                goto fail;
> +            }
>              ret = -TARGET_EFAULT;
> -            if (!(a = lock_user(VERIFY_READ, arg1,arg2, 0)))
> -                goto efault;
> -            if (!(p = lock_user_string(arg3)))
> +            p = lock_user_string(arg3);
> +            if (!p) {
>                  goto mincore_fail;
> +            }
>              ret = get_errno(mincore(a, arg2, p));
>              unlock_user(p, arg3, ret);
>              mincore_fail:
> 

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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.