diff mbox series

[v3,1/2] linux-user: Fix 'mq_timedsend()' and 'mq_timedreceive()'

Message ID 20200824193752.67950-2-Filip.Bozuta@syrmia.com (mailing list archive)
State New, archived
Headers show
Series linux-user: Introducing functionality for two time64 syscalls | expand

Commit Message

Filip Bozuta Aug. 24, 2020, 7:37 p.m. UTC
Implementations of syscalls 'mq_timedsend()' and 'mq_timedreceive()'
in 'syscall.c' use functions 'target_to_host_timespec()' and
'host_to_target_timespec()' to transfer the value of 'struct timespec'
between target and host. However, the implementations don't check whether
this conversion succeeds and thus can cause an unaproppriate error instead
of the 'EFAULT (Bad address)' which is supposed to be set if the conversion
from target to host fails. This was confirmed with the modified LTP
test suite where test cases with a bad adress for 'timespec' were
added. This modified test suite can be found at:
https://github.com/bozutaf/ltp

Without the changes from this patch the bad adress testcase for 'mq_timedsend()'
succeds unexpectedly, while the test returns errno 'ETIMEOUT' for
'mq_timedreceive()':

mq_timedsend01.c:190: FAIL: mq_timedsend() returned 0, expected -1: SUCCESS (0)
mq_timedreceive01.c:178: FAIL: mq_timedreceive() failed unexpectedly,
expected EFAULT: ETIMEDOUT (110)

After the changes from this patch, testcases for both syscalls fail with EFAULT
as expected, which is the same test result that is received with native execution:

mq_timedsend01.c:187: PASS: mq_timedsend() failed expectedly: EFAULT (14)
mq_timedreceive01.c:180: PASS: mq_timedreceive() failed expectedly: EFAULT (14)

(Patch with this new test case will be sent to LTP mailing list soon)

Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
---
 linux-user/syscall.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

Comments

Laurent Vivier Aug. 24, 2020, 8:15 p.m. UTC | #1
Le 24/08/2020 à 21:37, Filip Bozuta a écrit :
> Implementations of syscalls 'mq_timedsend()' and 'mq_timedreceive()'
> in 'syscall.c' use functions 'target_to_host_timespec()' and
> 'host_to_target_timespec()' to transfer the value of 'struct timespec'
> between target and host. However, the implementations don't check whether
> this conversion succeeds and thus can cause an unaproppriate error instead
> of the 'EFAULT (Bad address)' which is supposed to be set if the conversion
> from target to host fails. This was confirmed with the modified LTP
> test suite where test cases with a bad adress for 'timespec' were
> added. This modified test suite can be found at:
> https://github.com/bozutaf/ltp
> 
> Without the changes from this patch the bad adress testcase for 'mq_timedsend()'
> succeds unexpectedly, while the test returns errno 'ETIMEOUT' for
> 'mq_timedreceive()':
> 
> mq_timedsend01.c:190: FAIL: mq_timedsend() returned 0, expected -1: SUCCESS (0)
> mq_timedreceive01.c:178: FAIL: mq_timedreceive() failed unexpectedly,
> expected EFAULT: ETIMEDOUT (110)
> 
> After the changes from this patch, testcases for both syscalls fail with EFAULT
> as expected, which is the same test result that is received with native execution:
> 
> mq_timedsend01.c:187: PASS: mq_timedsend() failed expectedly: EFAULT (14)
> mq_timedreceive01.c:180: PASS: mq_timedreceive() failed expectedly: EFAULT (14)
> 
> (Patch with this new test case will be sent to LTP mailing list soon)
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> ---
>  linux-user/syscall.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 05f03919ff..4ee1de6e65 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -11817,9 +11817,13 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>  
>              p = lock_user (VERIFY_READ, arg2, arg3, 1);
>              if (arg5 != 0) {
> -                target_to_host_timespec(&ts, arg5);
> +                if (target_to_host_timespec(&ts, arg5)) {
> +                    return -TARGET_EFAULT;
> +                }
>                  ret = get_errno(safe_mq_timedsend(arg1, p, arg3, arg4, &ts));
> -                host_to_target_timespec(arg5, &ts);
> +                if (!is_error(ret) && host_to_target_timespec(arg5, &ts)) {
> +                    return -TARGET_EFAULT;
> +                }
>              } else {
>                  ret = get_errno(safe_mq_timedsend(arg1, p, arg3, arg4, NULL));
>              }
> @@ -11836,10 +11840,14 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>  
>              p = lock_user (VERIFY_READ, arg2, arg3, 1);
>              if (arg5 != 0) {
> -                target_to_host_timespec(&ts, arg5);
> +                if (target_to_host_timespec(&ts, arg5)) {
> +                    return -TARGET_EFAULT;
> +                }
>                  ret = get_errno(safe_mq_timedreceive(arg1, p, arg3,
>                                                       &prio, &ts));
> -                host_to_target_timespec(arg5, &ts);
> +                if (!is_error(ret) && host_to_target_timespec(arg5, &ts)) {
> +                    return -TARGET_EFAULT;
> +                }
>              } else {
>                  ret = get_errno(safe_mq_timedreceive(arg1, p, arg3,
>                                                       &prio, NULL));
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Laurent Vivier Aug. 24, 2020, 8:59 p.m. UTC | #2
Le 24/08/2020 à 21:37, Filip Bozuta a écrit :
> Implementations of syscalls 'mq_timedsend()' and 'mq_timedreceive()'
> in 'syscall.c' use functions 'target_to_host_timespec()' and
> 'host_to_target_timespec()' to transfer the value of 'struct timespec'
> between target and host. However, the implementations don't check whether
> this conversion succeeds and thus can cause an unaproppriate error instead
> of the 'EFAULT (Bad address)' which is supposed to be set if the conversion
> from target to host fails. This was confirmed with the modified LTP
> test suite where test cases with a bad adress for 'timespec' were
> added. This modified test suite can be found at:
> https://github.com/bozutaf/ltp
> 
> Without the changes from this patch the bad adress testcase for 'mq_timedsend()'
> succeds unexpectedly, while the test returns errno 'ETIMEOUT' for
> 'mq_timedreceive()':
> 
> mq_timedsend01.c:190: FAIL: mq_timedsend() returned 0, expected -1: SUCCESS (0)
> mq_timedreceive01.c:178: FAIL: mq_timedreceive() failed unexpectedly,
> expected EFAULT: ETIMEDOUT (110)
> 
> After the changes from this patch, testcases for both syscalls fail with EFAULT
> as expected, which is the same test result that is received with native execution:
> 
> mq_timedsend01.c:187: PASS: mq_timedsend() failed expectedly: EFAULT (14)
> mq_timedreceive01.c:180: PASS: mq_timedreceive() failed expectedly: EFAULT (14)
> 
> (Patch with this new test case will be sent to LTP mailing list soon)
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> ---
>  linux-user/syscall.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 05f03919ff..4ee1de6e65 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -11817,9 +11817,13 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>  
>              p = lock_user (VERIFY_READ, arg2, arg3, 1);
>              if (arg5 != 0) {
> -                target_to_host_timespec(&ts, arg5);
> +                if (target_to_host_timespec(&ts, arg5)) {
> +                    return -TARGET_EFAULT;
> +                }
>                  ret = get_errno(safe_mq_timedsend(arg1, p, arg3, arg4, &ts));
> -                host_to_target_timespec(arg5, &ts);
> +                if (!is_error(ret) && host_to_target_timespec(arg5, &ts)) {
> +                    return -TARGET_EFAULT;
> +                }
>              } else {
>                  ret = get_errno(safe_mq_timedsend(arg1, p, arg3, arg4, NULL));
>              }
> @@ -11836,10 +11840,14 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>  
>              p = lock_user (VERIFY_READ, arg2, arg3, 1);
>              if (arg5 != 0) {
> -                target_to_host_timespec(&ts, arg5);
> +                if (target_to_host_timespec(&ts, arg5)) {
> +                    return -TARGET_EFAULT;
> +                }
>                  ret = get_errno(safe_mq_timedreceive(arg1, p, arg3,
>                                                       &prio, &ts));
> -                host_to_target_timespec(arg5, &ts);
> +                if (!is_error(ret) && host_to_target_timespec(arg5, &ts)) {
> +                    return -TARGET_EFAULT;
> +                }
>              } else {
>                  ret = get_errno(safe_mq_timedreceive(arg1, p, arg3,
>                                                       &prio, NULL));
> 

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

Thanks,
Laurent
diff mbox series

Patch

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 05f03919ff..4ee1de6e65 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -11817,9 +11817,13 @@  static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
 
             p = lock_user (VERIFY_READ, arg2, arg3, 1);
             if (arg5 != 0) {
-                target_to_host_timespec(&ts, arg5);
+                if (target_to_host_timespec(&ts, arg5)) {
+                    return -TARGET_EFAULT;
+                }
                 ret = get_errno(safe_mq_timedsend(arg1, p, arg3, arg4, &ts));
-                host_to_target_timespec(arg5, &ts);
+                if (!is_error(ret) && host_to_target_timespec(arg5, &ts)) {
+                    return -TARGET_EFAULT;
+                }
             } else {
                 ret = get_errno(safe_mq_timedsend(arg1, p, arg3, arg4, NULL));
             }
@@ -11836,10 +11840,14 @@  static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
 
             p = lock_user (VERIFY_READ, arg2, arg3, 1);
             if (arg5 != 0) {
-                target_to_host_timespec(&ts, arg5);
+                if (target_to_host_timespec(&ts, arg5)) {
+                    return -TARGET_EFAULT;
+                }
                 ret = get_errno(safe_mq_timedreceive(arg1, p, arg3,
                                                      &prio, &ts));
-                host_to_target_timespec(arg5, &ts);
+                if (!is_error(ret) && host_to_target_timespec(arg5, &ts)) {
+                    return -TARGET_EFAULT;
+                }
             } else {
                 ret = get_errno(safe_mq_timedreceive(arg1, p, arg3,
                                                      &prio, NULL));