diff mbox series

linux-user: Fix syscall rt_sigtimedwait() implementation

Message ID 20200724181651.167819-1-Filip.Bozuta@syrmia.com
State New
Headers show
Series linux-user: Fix syscall rt_sigtimedwait() implementation | expand

Commit Message

Filip Bozuta July 24, 2020, 6:16 p.m. UTC 
Implementation of 'rt_sigtimedwait()' in 'syscall.c' uses the
function 'target_to_host_timespec()' to transfer the value of
'struct timespec' from target to host. However, the implementation
doesn'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 LTP test for rt_sigtimedwait:
"/testcases/kernel/syscalls/rt_sigtimedwait/rt_sigtimedwait01.c"
which causes an unapropriate error in test case "test_bad_adress3"
which is run with a bad adress for the 'struct timespec' argument:

FAIL: test_bad_address3 (349): Unexpected failure: EAGAIN/EWOULDBLOCK (11)

The test fails with an unexptected errno 'EAGAIN/EWOULDBLOCK' instead
of the expected EFAULT.

After the changes from this patch, the test case is executed successfully
along with the other LTP test cases for 'rt_sigtimedwait()':

PASS: test_bad_address3 (349): Test passed

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

Comments

Laurent Vivier July 24, 2020, 7:50 p.m. UTC | #1 
Le 24/07/2020 à 20:16, Filip Bozuta a écrit :
> Implementation of 'rt_sigtimedwait()' in 'syscall.c' uses the
> function 'target_to_host_timespec()' to transfer the value of
> 'struct timespec' from target to host. However, the implementation
> doesn'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 LTP test for rt_sigtimedwait:
> "/testcases/kernel/syscalls/rt_sigtimedwait/rt_sigtimedwait01.c"
> which causes an unapropriate error in test case "test_bad_adress3"
> which is run with a bad adress for the 'struct timespec' argument:
> 
> FAIL: test_bad_address3 (349): Unexpected failure: EAGAIN/EWOULDBLOCK (11)
> 
> The test fails with an unexptected errno 'EAGAIN/EWOULDBLOCK' instead
> of the expected EFAULT.
> 
> After the changes from this patch, the test case is executed successfully
> along with the other LTP test cases for 'rt_sigtimedwait()':
> 
> PASS: test_bad_address3 (349): Test passed
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> ---
>  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 1211e759c2..72735682cb 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -8868,7 +8868,9 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>              unlock_user(p, arg1, 0);
>              if (arg3) {
>                  puts = &uts;
> -                target_to_host_timespec(puts, arg3);
> +                if (target_to_host_timespec(puts, arg3)) {
> +                    return -TARGET_EFAULT;
> +                }
>              } else {
>                  puts = NULL;
>              }
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Laurent Vivier July 27, 2020, 8:04 p.m. UTC | #2 
Le 24/07/2020 à 20:16, Filip Bozuta a écrit :
> Implementation of 'rt_sigtimedwait()' in 'syscall.c' uses the
> function 'target_to_host_timespec()' to transfer the value of
> 'struct timespec' from target to host. However, the implementation
> doesn'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 LTP test for rt_sigtimedwait:
> "/testcases/kernel/syscalls/rt_sigtimedwait/rt_sigtimedwait01.c"
> which causes an unapropriate error in test case "test_bad_adress3"
> which is run with a bad adress for the 'struct timespec' argument:
> 
> FAIL: test_bad_address3 (349): Unexpected failure: EAGAIN/EWOULDBLOCK (11)
> 
> The test fails with an unexptected errno 'EAGAIN/EWOULDBLOCK' instead
> of the expected EFAULT.
> 
> After the changes from this patch, the test case is executed successfully
> along with the other LTP test cases for 'rt_sigtimedwait()':
> 
> PASS: test_bad_address3 (349): Test passed
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> ---
>  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 1211e759c2..72735682cb 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -8868,7 +8868,9 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>              unlock_user(p, arg1, 0);
>              if (arg3) {
>                  puts = &uts;
> -                target_to_host_timespec(puts, arg3);
> +                if (target_to_host_timespec(puts, arg3)) {
> +                    return -TARGET_EFAULT;
> +                }
>              } else {
>                  puts = NULL;
>              }
> 

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

Thanks,
Laurent
diff mbox series

Patch

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 1211e759c2..72735682cb 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8868,7 +8868,9 @@  static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
             unlock_user(p, arg1, 0);
             if (arg3) {
                 puts = &uts;
-                target_to_host_timespec(puts, arg3);
+                if (target_to_host_timespec(puts, arg3)) {
+                    return -TARGET_EFAULT;
+                }
             } else {
                 puts = NULL;
             }