Implementation of syscall 'utimensat()' in 'syscall.c' uses functions
target_to_host/host_to_target_timespec() to convert values of
'struct timespec' between host and target. However, the implementation
doesn't check whether the conversion succeeds and thus can cause an
inappropriate error or succeed unappropriately instead of setting errno
EFAULT ('Bad address') which is supposed to be set in these cases.

This was confirmed with the LTP test for utimensat ('testcases/utimensat')
which fails for test cases when the errno EFAULT is expected. After changes
from this patch, the test passes for all test cases.

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

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 05f03919ff..920656191b 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -11722,8 +11722,13 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
             if (!arg3) {
                 tsp = NULL;
             } else {
-                target_to_host_timespec(ts, arg3);
-                target_to_host_timespec(ts+1, arg3+sizeof(struct target_timespec));
+                if (target_to_host_timespec(ts, arg3)) {
+                    return -TARGET_EFAULT;
+                }
+                if (target_to_host_timespec(ts + 1, arg3 +
+                                            sizeof(struct target_timespec))) {
+                    return -TARGET_EFAULT;
+                }
                 tsp = ts;
             }
             if (!arg2)
-- 
2.25.1

Le 11/08/2020 à 13:31, Filip Bozuta a écrit :
> Implementation of syscall 'utimensat()' in 'syscall.c' uses functions
> target_to_host/host_to_target_timespec() to convert values of
> 'struct timespec' between host and target. However, the implementation
> doesn't check whether the conversion succeeds and thus can cause an
> inappropriate error or succeed unappropriately instead of setting errno
> EFAULT ('Bad address') which is supposed to be set in these cases.
> 
> This was confirmed with the LTP test for utimensat ('testcases/utimensat')
> which fails for test cases when the errno EFAULT is expected. After changes
> from this patch, the test passes for all test cases.
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> ---
>  linux-user/syscall.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 05f03919ff..920656191b 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -11722,8 +11722,13 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>              if (!arg3) {
>                  tsp = NULL;
>              } else {
> -                target_to_host_timespec(ts, arg3);
> -                target_to_host_timespec(ts+1, arg3+sizeof(struct target_timespec));
> +                if (target_to_host_timespec(ts, arg3)) {
> +                    return -TARGET_EFAULT;
> +                }
> +                if (target_to_host_timespec(ts + 1, arg3 +
> +                                            sizeof(struct target_timespec))) {
> +                    return -TARGET_EFAULT;
> +                }
>                  tsp = ts;
>              }
>              if (!arg2)
> 

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

Le 11/08/2020 à 13:31, Filip Bozuta a écrit :
> Implementation of syscall 'utimensat()' in 'syscall.c' uses functions
> target_to_host/host_to_target_timespec() to convert values of
> 'struct timespec' between host and target. However, the implementation
> doesn't check whether the conversion succeeds and thus can cause an
> inappropriate error or succeed unappropriately instead of setting errno
> EFAULT ('Bad address') which is supposed to be set in these cases.
> 
> This was confirmed with the LTP test for utimensat ('testcases/utimensat')
> which fails for test cases when the errno EFAULT is expected. After changes
> from this patch, the test passes for all test cases.
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> ---
>  linux-user/syscall.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 05f03919ff..920656191b 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -11722,8 +11722,13 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>              if (!arg3) {
>                  tsp = NULL;
>              } else {
> -                target_to_host_timespec(ts, arg3);
> -                target_to_host_timespec(ts+1, arg3+sizeof(struct target_timespec));
> +                if (target_to_host_timespec(ts, arg3)) {
> +                    return -TARGET_EFAULT;
> +                }
> +                if (target_to_host_timespec(ts + 1, arg3 +
> +                                            sizeof(struct target_timespec))) {
> +                    return -TARGET_EFAULT;
> +                }
>                  tsp = ts;
>              }
>              if (!arg2)
> 

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

Thanks,
Laurent