diff mbox series

[1/2] linux-user: Modify 'target_to_host/host_to_target_itimerspec()'

Message ID 20200722111024.272553-2-Filip.Bozuta@syrmia.com
State New
Headers show
Series Adding support for a group of year 2038 safe syscalls | expand

Commit Message

Filip Bozuta July 22, 2020, 11:10 a.m. UTC 
Functions 'target_to_host_itimerspec()' and 'host_to_target_itimerspec()'
are used to convert values of type 'struct itimerspec' between target and
host. This type has 'struct timespec' as its fields. That is the reason
why this patch introduces a little modification to the converting functions
to be implemented using already existing functions that convert 'struct timespec':
'target_to_host_timespec()' and 'host_to_target_timespec()'. This makes the
code of 'target_to_host_itimerspec()' and 'host_to_target_itimerspec()' more
clean and readable.

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

Comments

Laurent Vivier July 22, 2020, 1:26 p.m. UTC | #1 
Le 22/07/2020 à 13:10, Filip Bozuta a écrit :
> Functions 'target_to_host_itimerspec()' and 'host_to_target_itimerspec()'
> are used to convert values of type 'struct itimerspec' between target and
> host. This type has 'struct timespec' as its fields. That is the reason
> why this patch introduces a little modification to the converting functions
> to be implemented using already existing functions that convert 'struct timespec':
> 'target_to_host_timespec()' and 'host_to_target_timespec()'. This makes the
> code of 'target_to_host_itimerspec()' and 'host_to_target_itimerspec()' more
> clean and readable.
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> ---
>  linux-user/syscall.c | 38 ++++++++++++--------------------------
>  1 file changed, 12 insertions(+), 26 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1211e759c2..17e09c3cd3 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1236,7 +1236,9 @@ static inline abi_long copy_to_user_timeval64(abi_ulong target_tv_addr,
>      defined(TARGET_NR_nanosleep) || defined(TARGET_NR_clock_settime) || \
>      defined(TARGET_NR_utimensat) || defined(TARGET_NR_mq_timedsend) || \
>      defined(TARGET_NR_mq_timedreceive) || defined(TARGET_NR_ipc) || \
> -    defined(TARGET_NR_semop) || defined(TARGET_NR_semtimedop)
> +    defined(TARGET_NR_semop) || defined(TARGET_NR_semtimedop) || \
> +    defined(TARGET_NR_timer_settime) || \
> +    (defined(TARGET_NR_timerfd_settime) && defined(CONFIG_TIMERFD))
>  static inline abi_long target_to_host_timespec(struct timespec *host_ts,
>                                                 abi_ulong target_addr)
>  {
> @@ -6790,46 +6792,30 @@ static inline abi_long target_ftruncate64(void *cpu_env, abi_long arg1,
>  
>  #if defined(TARGET_NR_timer_settime) || \
>      (defined(TARGET_NR_timerfd_settime) && defined(CONFIG_TIMERFD))
> -static inline abi_long target_to_host_itimerspec(struct itimerspec *host_itspec,
> +static inline abi_long target_to_host_itimerspec(struct itimerspec *host_its,
>                                                   abi_ulong target_addr)
>  {
> -    struct target_itimerspec *target_itspec;
> -
> -    if (!lock_user_struct(VERIFY_READ, target_itspec, target_addr, 1)) {
> +    if (target_to_host_timespec(&host_its->it_interval, target_addr) ||
> +        target_to_host_timespec(&host_its->it_value, target_addr +
> +                                sizeof(struct target_timespec))) {

The "+ sizeof(struct target_timespec)" ignores possible type alignment
in the structure.

I think it is safer to use "+ offsetof(struct target_timespec,
it_interval)" for the first one and "+ offsetof(struct target_timespec,
it_value)" for the second one.

Thanks,
Laurent
diff mbox series

Patch

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 1211e759c2..17e09c3cd3 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1236,7 +1236,9 @@  static inline abi_long copy_to_user_timeval64(abi_ulong target_tv_addr,
     defined(TARGET_NR_nanosleep) || defined(TARGET_NR_clock_settime) || \
     defined(TARGET_NR_utimensat) || defined(TARGET_NR_mq_timedsend) || \
     defined(TARGET_NR_mq_timedreceive) || defined(TARGET_NR_ipc) || \
-    defined(TARGET_NR_semop) || defined(TARGET_NR_semtimedop)
+    defined(TARGET_NR_semop) || defined(TARGET_NR_semtimedop) || \
+    defined(TARGET_NR_timer_settime) || \
+    (defined(TARGET_NR_timerfd_settime) && defined(CONFIG_TIMERFD))
 static inline abi_long target_to_host_timespec(struct timespec *host_ts,
                                                abi_ulong target_addr)
 {
@@ -6790,46 +6792,30 @@  static inline abi_long target_ftruncate64(void *cpu_env, abi_long arg1,
 
 #if defined(TARGET_NR_timer_settime) || \
     (defined(TARGET_NR_timerfd_settime) && defined(CONFIG_TIMERFD))
-static inline abi_long target_to_host_itimerspec(struct itimerspec *host_itspec,
+static inline abi_long target_to_host_itimerspec(struct itimerspec *host_its,
                                                  abi_ulong target_addr)
 {
-    struct target_itimerspec *target_itspec;
-
-    if (!lock_user_struct(VERIFY_READ, target_itspec, target_addr, 1)) {
+    if (target_to_host_timespec(&host_its->it_interval, target_addr) ||
+        target_to_host_timespec(&host_its->it_value, target_addr +
+                                sizeof(struct target_timespec))) {
         return -TARGET_EFAULT;
     }
 
-    host_itspec->it_interval.tv_sec =
-                            tswapal(target_itspec->it_interval.tv_sec);
-    host_itspec->it_interval.tv_nsec =
-                            tswapal(target_itspec->it_interval.tv_nsec);
-    host_itspec->it_value.tv_sec = tswapal(target_itspec->it_value.tv_sec);
-    host_itspec->it_value.tv_nsec = tswapal(target_itspec->it_value.tv_nsec);
-
-    unlock_user_struct(target_itspec, target_addr, 1);
     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)
+      defined(TARGET_NR_timer_gettime) || defined(TARGET_NR_timer_settime)
 static inline abi_long host_to_target_itimerspec(abi_ulong target_addr,
-                                               struct itimerspec *host_its)
+                                                 struct itimerspec *host_its)
 {
-    struct target_itimerspec *target_itspec;
-
-    if (!lock_user_struct(VERIFY_WRITE, target_itspec, target_addr, 0)) {
+    if (host_to_target_timespec(target_addr, &host_its->it_interval) ||
+        host_to_target_timespec(target_addr + sizeof(struct target_timespec),
+                                &host_its->it_value)) {
         return -TARGET_EFAULT;
     }
-
-    target_itspec->it_interval.tv_sec = tswapal(host_its->it_interval.tv_sec);
-    target_itspec->it_interval.tv_nsec = tswapal(host_its->it_interval.tv_nsec);
-
-    target_itspec->it_value.tv_sec = tswapal(host_its->it_value.tv_sec);
-    target_itspec->it_value.tv_nsec = tswapal(host_its->it_value.tv_nsec);
-
-    unlock_user_struct(target_itspec, target_addr, 0);
     return 0;
 }
 #endif