On Saturday 16 May 2015 00:46:44 Thomas Gleixner wrote:
> On Wed, 6 May 2015, Arnd Bergmann wrote:
> > +SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops,
> > +           unsigned, nsops,
> > +           const struct __kernel_timespec  __user *, timeout)
> > +{
> > +   unsigned long jiffies_left = 0;
> > +
> > +   if (timeout) {
> > +           struct timespec64 _timeout;
> > +           if (get_timespec64(&_timeout, timeout))
> 
> Moo. I had to look 3 times to get not confused by the extra
> underscore. What's wrong with a proper variable name which is easy to
> distinguish?
> 
> > +                   return -EFAULT;
> 
> > +           if (_timeout.tv_sec < 0 || _timeout.tv_nsec < 0 ||
> > +                   _timeout.tv_nsec >= 1000000000L)
> > +                   return -EINVAL;
> 
> We have proper helper functions to validate time specs.

I ended up fixing both issues you noticed in the same patch
after all, and also simplified it slightly more.

Finally, I also noticed that I had not done a timespec64_to_jiffies()
call at the time when I wrote this patch, but it actually exists now,
so I've reordered my series and am using it in the new version, as
I should have done to start with.

        Arnd

8<----

>From e04b14d49273c27d92f1799233b82bcdafb43d9a Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <[email protected]>
Date: Mon, 27 Apr 2015 23:30:39 +0200
Subject: [UPDATED PATCH] y2038: add compat handling for sys_semtimedop

This moves the compat_sys_semtimedop function to ipc/sem.c so it
can be shared with 32-bit architectures efficiently. Instead of
copying the timespec back to user space, we take a shortcut and
pass the kernel timespec64 value to the low-level implementation
directly.

The native sys_semtimedop() function is modified to take a
__kernel_timespec structure, which will be based on a 64-bit
time_t in the future.

There is a small API change here: if multiple errors are present,
and the timespec argument is an invalid pointer, we now return
-EFAULT before checking any of the other error conditions.
This is what the compat version has always done, but if it is a
problem, we need a more sophisticated approach.

Signed-off-by: Arnd Bergmann <[email protected]>

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index f3fdc312627b..c2a70a8f907d 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -665,7 +665,7 @@ asmlinkage long sys_semop(int semid, struct sembuf __user 
*sops,
 asmlinkage long sys_semctl(int semid, int semnum, int cmd, unsigned long arg);
 asmlinkage long sys_semtimedop(int semid, struct sembuf __user *sops,
                                unsigned nsops,
-                               const struct timespec __user *timeout);
+                               const struct __kernel_timespec __user *timeout);
 asmlinkage long sys_shmat(int shmid, char __user *shmaddr, int shmflg);
 asmlinkage long sys_shmget(key_t key, size_t size, int flag);
 asmlinkage long sys_shmdt(char __user *shmaddr);
diff --git a/ipc/compat.c b/ipc/compat.c
index 9b3c85f8a538..2bbdb093d1be 100644
--- a/ipc/compat.c
+++ b/ipc/compat.c
@@ -745,13 +745,3 @@ COMPAT_SYSCALL_DEFINE3(shmctl, int, first, int, second, 
void __user *, uptr)
        }
        return err;
 }
-
-COMPAT_SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsems,
-                      unsigned, nsops,
-                      const struct compat_timespec __user *, timeout)
-{
-       struct timespec __user *ts64;
-       if (compat_convert_timespec(&ts64, timeout))
-               return -EFAULT;
-       return sys_semtimedop(semid, tsems, nsops, ts64);
-}
diff --git a/ipc/sem.c b/ipc/sem.c
index d1a6edd17eba..84d354a34df3 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -72,6 +72,7 @@
  *   The worst-case behavior is nevertheless O(N^2) for N wakeups.
  */
 
+#include <linux/compat.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/init.h>
@@ -1779,8 +1780,8 @@ static int get_queue_result(struct sem_queue *q)
        return error;
 }
 
-SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops,
-               unsigned, nsops, const struct timespec __user *, timeout)
+static long semtimedop(int semid, struct sembuf __user * tsops,
+                      unsigned nsops, struct timespec64 *timeout)
 {
        int error = -EINVAL;
        struct sem_array *sma;
@@ -1809,17 +1810,11 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf 
__user *, tsops,
                goto out_free;
        }
        if (timeout) {
-               struct timespec _timeout;
-               if (copy_from_user(&_timeout, timeout, sizeof(*timeout))) {
-                       error = -EFAULT;
-                       goto out_free;
-               }
-               if (_timeout.tv_sec < 0 || _timeout.tv_nsec < 0 ||
-                       _timeout.tv_nsec >= 1000000000L) {
+               if (!timespec64_valid(timeout)) {
                        error = -EINVAL;
                        goto out_free;
                }
-               jiffies_left = timespec_to_jiffies(&_timeout);
+               jiffies_left = timespec64_to_jiffies(timeout);
        }
        max = 0;
        for (sop = sops; sop < sops + nsops; sop++) {
@@ -2014,10 +2009,36 @@ out_free:
        return error;
 }
 
+SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops,
+               unsigned, nsops,
+               const struct __kernel_timespec  __user *, p)
+{
+       struct timespec64 timeout;
+
+       if (p && get_timespec64(&timeout, p))
+               return -EFAULT;
+
+       return semtimedop(semid, tsops, nsops, p ? &timeout : NULL);
+}
+
+#ifdef CONFIG_COMPAT_TIME
+COMPAT_SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops,
+               unsigned, nsops,
+               const struct compat_timespec  __user *, p)
+{
+       struct timespec64 timeout;
+
+       if (p && compat_get_timespec64(&timeout, p))
+               return -EFAULT;
+
+       return semtimedop(semid, tsops, nsops, p ? &timeout : NULL);
+}
+#endif
+
 SYSCALL_DEFINE3(semop, int, semid, struct sembuf __user *, tsops,
                unsigned, nsops)
 {
-       return sys_semtimedop(semid, tsops, nsops, NULL);
+       return semtimedop(semid, tsops, nsops, NULL);
 }
 
 /* If CLONE_SYSVSEM is set, establish sharing of SEM_UNDO state between
diff --git a/ipc/syscall.c b/ipc/syscall.c
index 52429489cde0..d7b17355d870 100644
--- a/ipc/syscall.c
+++ b/ipc/syscall.c
@@ -7,6 +7,7 @@
 #include <linux/unistd.h>
 
 #ifdef __ARCH_WANT_SYS_IPC
+#include <linux/compat_time.h>
 #include <linux/errno.h>
 #include <linux/ipc.h>
 #include <linux/shm.h>
@@ -26,9 +27,15 @@ SYSCALL_DEFINE6(ipc, unsigned int, call, int, first, 
unsigned long, second,
                return sys_semtimedop(first, (struct sembuf __user *)ptr,
                                      second, NULL);
        case SEMTIMEDOP:
+#if defined(CONFIG_ARCH_HAS_COMPAT_TIME) && !defined(CONFIG_64BIT)
+               return compat_sys_semtimedop(first, (struct sembuf __user *)ptr,
+                                            second,
+                                            (const struct compat_timespec 
__user *)fifth);
+#else
                return sys_semtimedop(first, (struct sembuf __user *)ptr,
                                      second,
                                      (const struct timespec __user *)fifth);
+#endif
 
        case SEMGET:
                return sys_semget(first, second, third);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to