Re: [PATCH v2 2/2] linux-user: Add support for a group of 2038 safe syscalls

[Laurent Vivier]

Le 22/07/2020 à 17:34, Filip Bozuta a écrit :
> This patch implements functionality for following time64 syscalls:
> 
> *clock_getres_time64
> 
>      This a year 2038 safe variant of syscall:
> 
>      int clock_getres(clockid_t clockid, struct timespec *res)
>      --finding the resoultion of a specified clock--
>      man page: https://man7.org/linux/man-pages/man2/clock_getres.2.html
> 
> *timer_gettime64
> *timer_settime64
> 
>      These are year 2038 safe variants of syscalls:
> 
>      int timer_settime(timer_t timerid, int flags,
>                        const struct itimerspec *new_value,
>                        struct itimerspec *old_value)
>      int timer_gettime(timer_t timerid, struct itimerspec *curr_value)
>      --arming/dissarming and fetching state of POSIX per-process timer--
>      man page: https://man7.org/linux/man-pages/man2/timer_settime.2.html
> 
> *timerfd_gettime64
> *timerfd_settime64
> 
>      These are year 2038 safe variants of syscalls:
> 
>      int timerfd_settime(int fd, int flags,
>                          const struct itimerspec *new_value,
>                          struct itimerspec *old_value)
>      int timerfd_gettime(int fd, struct itimerspec *curr_value)
>      --timers that notify via file descriptor--
>      man page: https://man7.org/linux/man-pages/man2/timerfd_settime.2.html
> 
> Implementation notes:
> 
>      Syscall 'clock_getres_time64' was implemented similarly to 
> 'clock_getres()'.
>      The only difference was that for the conversion of 'struct timespec' from
>      host to target, function 'host_to_target_timespec64()' was used instead 
> of
>      'host_to_target_timespec()'.
> 
>      For other syscalls, new functions 'host_to_target_itimerspec64()' and
>      'target_to_host_itimerspec64()' were added to convert the value of the
>      'struct itimerspec' from host to target and vice versa. A new type
>      'struct target__kernel_itimerspec' was added in 'syscall_defs.h'. This
>      type was defined with fields which are of the already defined type
>      'struct target_timespec'. This new 'struct target__kernel_itimerspec'
>      type is used in these new converting functions. These new functions were
>      defined similarly to 'host_to_target_itimerspec()' and 
> 'target_to_host_itimerspec()'
>      the only difference being that 'target_to_host_timespec64()' and
>      'host_to_target_timespec64()' were used.
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> ---
>  linux-user/syscall.c      | 139 +++++++++++++++++++++++++++++++++++++-
>  linux-user/syscall_defs.h |   5 ++
>  2 files changed, 143 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index b1baed346c..9040e794ec 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1254,7 +1254,9 @@ static inline abi_long target_to_host_timespec(struct 
> timespec *host_ts,
>  }
>  #endif
>  
> -#if defined(TARGET_NR_clock_settime64) || defined(TARGET_NR_futex_time64)
> +#if defined(TARGET_NR_clock_settime64) || defined(TARGET_NR_futex_time64) || 
> \
> +    defined(TARGET_NR_timer_settime64) || \
> +    (defined(TARGET_NR_timerfd_settime64) && defined(CONFIG_TIMERFD))
>  static inline abi_long target_to_host_timespec64(struct timespec *host_ts,
>                                                   abi_ulong target_addr)
>  {
> @@ -6808,6 +6810,24 @@ static inline abi_long 
> target_to_host_itimerspec(struct itimerspec *host_its,
>  }
>  #endif
>  
> +#if defined(TARGET_NR_timer_settime64) || \
> +    (defined(TARGET_NR_timerfd_settime64) && defined(CONFIG_TIMERFD))
> +static inline abi_long target_to_host_itimerspec64(struct itimerspec 
> *host_its,
> +                                                   abi_ulong target_addr)
> +{
> +    if (target_to_host_timespec64(&host_its->it_interval, target_addr +
> +                                  offsetof(struct target__kernel_itimerspec,
> +                                           it_interval)) ||
> +        target_to_host_timespec64(&host_its->it_value, target_addr +
> +                                  offsetof(struct target__kernel_itimerspec,
> +                                           it_value))) {
> +        return -TARGET_EFAULT;
> +    }
> +
> +    return 0;
> +}
> +#endif
> +
>  #if ((defined(TARGET_NR_timerfd_gettime) || \
>        defined(TARGET_NR_timerfd_settime)) && defined(CONFIG_TIMERFD)) || \
>        defined(TARGET_NR_timer_gettime) || defined(TARGET_NR_timer_settime)
> @@ -6826,6 +6846,26 @@ static inline abi_long 
> host_to_target_itimerspec(abi_ulong target_addr,
>  }
>  #endif
>  
> +#if ((defined(TARGET_NR_timerfd_gettime64) || \
> +      defined(TARGET_NR_timerfd_settime64)) && defined(CONFIG_TIMERFD)) || \
> +      defined(TARGET_NR_timer_gettime64) || 
> defined(TARGET_NR_timer_settime64)
> +static inline abi_long host_to_target_itimerspec64(abi_ulong target_addr,
> +                                                   struct itimerspec 
> *host_its)
> +{
> +    if (host_to_target_timespec64(target_addr +
> +                                  offsetof(struct target__kernel_itimerspec,
> +                                           it_interval),
> +                                  &host_its->it_interval) ||
> +        host_to_target_timespec64(target_addr +
> +                                  offsetof(struct target__kernel_itimerspec,
> +                                           it_value),
> +                                  &host_its->it_value)) {
> +        return -TARGET_EFAULT;
> +    }
> +    return 0;
> +}
> +#endif
> +
>  #if defined(TARGET_NR_adjtimex) || \
>      (defined(TARGET_NR_clock_adjtime) && defined(CONFIG_CLOCK_ADJTIME))
>  static inline abi_long target_to_host_timex(struct timex *host_tx,
> @@ -11816,6 +11856,17 @@ static abi_long do_syscall1(void *cpu_env, int num, 
> abi_long arg1,
>          return ret;
>      }
>  #endif
> +#ifdef TARGET_NR_clock_getres_time64
> +    case TARGET_NR_clock_getres_time64:
> +    {
> +        struct timespec ts;
> +        ret = get_errno(clock_getres(arg1, &ts));
> +        if (!is_error(ret)) {
> +            host_to_target_timespec64(arg2, &ts);
> +        }
> +        return ret;
> +    }
> +#endif
>  #ifdef TARGET_NR_clock_nanosleep
>      case TARGET_NR_clock_nanosleep:
>      {
> @@ -12411,6 +12462,32 @@ static abi_long do_syscall1(void *cpu_env, int num, 
> abi_long arg1,
>      }
>  #endif
>  
> +#ifdef TARGET_NR_timer_settime64
> +    case TARGET_NR_timer_settime64:
> +    {
> +        target_timer_t timerid = get_timer_id(arg1);
> +
> +        if (timerid < 0) {
> +            ret = timerid;
> +        } else if (arg3 == 0) {
> +            ret = -TARGET_EINVAL;
> +        } else {
> +            timer_t htimer = g_posix_timers[timerid];
> +            struct itimerspec hspec_new = {{0},}, hspec_old = {{0},};
> +
> +            if (target_to_host_itimerspec64(&hspec_new, arg3)) {
> +                return -TARGET_EFAULT;
> +            }
> +            ret = get_errno(
> +                          timer_settime(htimer, arg2, &hspec_new, 
> &hspec_old));
> +            if (arg4 && host_to_target_itimerspec64(arg4, &hspec_old)) {
> +                return -TARGET_EFAULT;
> +            }
> +        }
> +        return ret;
> +    }
> +#endif
> +
>  #ifdef TARGET_NR_timer_gettime
>      case TARGET_NR_timer_gettime:
>      {
> @@ -12434,6 +12511,29 @@ static abi_long do_syscall1(void *cpu_env, int num, 
> abi_long arg1,
>      }
>  #endif
>  
> +#ifdef TARGET_NR_timer_gettime64
> +    case TARGET_NR_timer_gettime64:
> +    {
> +        /* args: timer_t timerid, struct itimerspec64 *curr_value */
> +        target_timer_t timerid = get_timer_id(arg1);
> +
> +        if (timerid < 0) {
> +            ret = timerid;
> +        } else if (!arg2) {
> +            ret = -TARGET_EFAULT;
> +        } else {
> +            timer_t htimer = g_posix_timers[timerid];
> +            struct itimerspec hspec;
> +            ret = get_errno(timer_gettime(htimer, &hspec));
> +
> +            if (host_to_target_itimerspec64(arg2, &hspec)) {
> +                ret = -TARGET_EFAULT;
> +            }
> +        }
> +        return ret;
> +    }
> +#endif
> +
>  #ifdef TARGET_NR_timer_getoverrun
>      case TARGET_NR_timer_getoverrun:
>      {
> @@ -12487,6 +12587,20 @@ static abi_long do_syscall1(void *cpu_env, int num, 
> abi_long arg1,
>          return ret;
>  #endif
>  
> +#if defined(TARGET_NR_timerfd_gettime64) && defined(CONFIG_TIMERFD)
> +    case TARGET_NR_timerfd_gettime64:
> +        {
> +            struct itimerspec its_curr;
> +
> +            ret = get_errno(timerfd_gettime(arg1, &its_curr));
> +
> +            if (arg2 && host_to_target_itimerspec64(arg2, &its_curr)) {
> +                return -TARGET_EFAULT;
> +            }
> +        }
> +        return ret;
> +#endif
> +
>  #if defined(TARGET_NR_timerfd_settime) && defined(CONFIG_TIMERFD)
>      case TARGET_NR_timerfd_settime:
>          {
> @@ -12510,6 +12624,29 @@ static abi_long do_syscall1(void *cpu_env, int num, 
> abi_long arg1,
>          return ret;
>  #endif
>  
> +#if defined(TARGET_NR_timerfd_settime64) && defined(CONFIG_TIMERFD)
> +    case TARGET_NR_timerfd_settime64:
> +        {
> +            struct itimerspec its_new, its_old, *p_new;
> +
> +            if (arg3) {
> +                if (target_to_host_itimerspec64(&its_new, arg3)) {
> +                    return -TARGET_EFAULT;
> +                }
> +                p_new = &its_new;
> +            } else {
> +                p_new = NULL;
> +            }
> +
> +            ret = get_errno(timerfd_settime(arg1, arg2, p_new, &its_old));
> +
> +            if (arg4 && host_to_target_itimerspec64(arg4, &its_old)) {
> +                return -TARGET_EFAULT;
> +            }
> +        }
> +        return ret;
> +#endif
> +
>  #if defined(TARGET_NR_ioprio_get) && defined(__NR_ioprio_get)
>      case TARGET_NR_ioprio_get:
>          return get_errno(ioprio_get(arg1, arg2));
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 3c261cff0e..427a25f5bc 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -259,6 +259,11 @@ struct target_itimerspec {
>      struct target_timespec it_value;
>  };
>  
> +struct target__kernel_itimerspec {
> +    struct target__kernel_timespec it_interval;
> +    struct target__kernel_timespec it_value;
> +};
> +
>  struct target_timex {
>      abi_uint modes;              /* Mode selector */
>      abi_long offset;             /* Time offset */
> 

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

Thanks,
Laurent