ARM: 9114/1: oabi-compat: rework sys_semtimedop emulation

sys_oabi_semtimedop() is one of the last users of set_fs() on Arm. To
remove this one, expose the internal code of the actual implementation
that operates on a kernel pointer and call it directly after copying.

There should be no measurable impact on the normal execution of this
function, and it makes the overly long function a little shorter, which
may help readability.

While reworking the oabi version, make it behave a little more like
the native one, using kvmalloc_array() and restructure the code
flow in a similar way.

The naming of __do_semtimedop() is not very good, I hope someone can
come up with a better name.

One regression was spotted by kernel test robot <rong.a.chen@intel.com>
and fixed before the first mailing list submission.

Acked-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

diff --git a/arch/arm/kernel/sys_oabi-compat.c b/arch/arm/kernel/sys_oabi-compat.c
index 1f6a433..5ea365c 100644 (file)
--- a/arch/arm/kernel/sys_oabi-compat.c
+++ b/arch/arm/kernel/sys_oabi-compat.c
@@ -80,6 +80,7 @@
 #include <linux/socket.h>
 #include <linux/net.h>
 #include <linux/ipc.h>
+#include <linux/ipc_namespace.h>
 #include <linux/uaccess.h>
 #include <linux/slab.h>
 
@@ -302,46 +303,52 @@ struct oabi_sembuf {
        unsigned short  __pad;
 };
 
+#define sc_semopm     sem_ctls[2]
+
+#ifdef CONFIG_SYSVIPC
 asmlinkage long sys_oabi_semtimedop(int semid,
                                    struct oabi_sembuf __user *tsops,
                                    unsigned nsops,
                                    const struct old_timespec32 __user *timeout)
 {
+       struct ipc_namespace *ns;
        struct sembuf *sops;
-       struct old_timespec32 local_timeout;
        long err;
        int i;
 
+       ns = current->nsproxy->ipc_ns;
+       if (nsops > ns->sc_semopm)
+               return -E2BIG;
        if (nsops < 1 || nsops > SEMOPM)
                return -EINVAL;
-       if (!access_ok(tsops, sizeof(*tsops) * nsops))
-               return -EFAULT;
-       sops = kmalloc_array(nsops, sizeof(*sops), GFP_KERNEL);
+       sops = kvmalloc_array(nsops, sizeof(*sops), GFP_KERNEL);
        if (!sops)
                return -ENOMEM;
        err = 0;
        for (i = 0; i < nsops; i++) {
                struct oabi_sembuf osb;
-               err |= __copy_from_user(&osb, tsops, sizeof(osb));
+               err |= copy_from_user(&osb, tsops, sizeof(osb));
                sops[i].sem_num = osb.sem_num;
                sops[i].sem_op = osb.sem_op;
                sops[i].sem_flg = osb.sem_flg;
                tsops++;
        }
-       if (timeout) {
-               /* copy this as well before changing domain protection */
-               err |= copy_from_user(&local_timeout, timeout, sizeof(*timeout));
-               timeout = &local_timeout;
-       }
        if (err) {
                err = -EFAULT;
-       } else {
-               mm_segment_t fs = get_fs();
-               set_fs(KERNEL_DS);
-               err = sys_semtimedop_time32(semid, sops, nsops, timeout);
-               set_fs(fs);
+               goto out;
+       }
+
+       if (timeout) {
+               struct timespec64 ts;
+               err = get_old_timespec32(&ts, timeout);
+               if (err)
+                       goto out;
+               err = __do_semtimedop(semid, sops, nsops, &ts, ns);
+               goto out;
        }
-       kfree(sops);
+       err = __do_semtimedop(semid, sops, nsops, NULL, ns);
+out:
+       kvfree(sops);
        return err;
 }
 
@@ -368,6 +375,27 @@ asmlinkage int sys_oabi_ipc(uint call, int first, int second, int third,
                return sys_ipc(call, first, second, third, ptr, fifth);
        }
 }
+#else
+asmlinkage long sys_oabi_semtimedop(int semid,
+                                   struct oabi_sembuf __user *tsops,
+                                   unsigned nsops,
+                                   const struct old_timespec32 __user *timeout)
+{
+       return -ENOSYS;
+}
+
+asmlinkage long sys_oabi_semop(int semid, struct oabi_sembuf __user *tsops,
+                              unsigned nsops)
+{
+       return -ENOSYS;
+}
+
+asmlinkage int sys_oabi_ipc(uint call, int first, int second, int third,
+                           void __user *ptr, long fifth)
+{
+       return -ENOSYS;
+}
+#endif
 
 asmlinkage long sys_oabi_bind(int fd, struct sockaddr __user *addr, int addrlen)
 {

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 69c9a70..6c6fc3f 100644 (file)
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -1373,6 +1373,9 @@ long ksys_old_shmctl(int shmid, int cmd, struct shmid_ds __user *buf);
 long compat_ksys_semtimedop(int semid, struct sembuf __user *tsems,
                            unsigned int nsops,
                            const struct old_timespec32 __user *timeout);
+long __do_semtimedop(int semid, struct sembuf *tsems, unsigned int nsops,
+                    const struct timespec64 *timeout,
+                    struct ipc_namespace *ns);
 
 int __sys_getsockopt(int fd, int level, int optname, char __user *optval,
                int __user *optlen);

diff --git a/ipc/sem.c b/ipc/sem.c
index 971e75d..ae8d910 100644 (file)
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -1984,46 +1984,34 @@ out:
        return un;
 }
 
-static long do_semtimedop(int semid, struct sembuf __user *tsops,
-               unsigned nsops, const struct timespec64 *timeout)
+long __do_semtimedop(int semid, struct sembuf *sops,
+               unsigned nsops, const struct timespec64 *timeout,
+               struct ipc_namespace *ns)
 {
        int error = -EINVAL;
        struct sem_array *sma;
-       struct sembuf fast_sops[SEMOPM_FAST];
-       struct sembuf *sops = fast_sops, *sop;
+       struct sembuf *sop;
        struct sem_undo *un;
        int max, locknum;
        bool undos = false, alter = false, dupsop = false;
        struct sem_queue queue;
        unsigned long dup = 0, jiffies_left = 0;
-       struct ipc_namespace *ns;
-
-       ns = current->nsproxy->ipc_ns;
 
        if (nsops < 1 || semid < 0)
                return -EINVAL;
        if (nsops > ns->sc_semopm)
                return -E2BIG;
-       if (nsops > SEMOPM_FAST) {
-               sops = kvmalloc_array(nsops, sizeof(*sops), GFP_KERNEL);
-               if (sops == NULL)
-                       return -ENOMEM;
-       }
-
-       if (copy_from_user(sops, tsops, nsops * sizeof(*tsops))) {
-               error =  -EFAULT;
-               goto out_free;
-       }
 
        if (timeout) {
                if (timeout->tv_sec < 0 || timeout->tv_nsec < 0 ||
                        timeout->tv_nsec >= 1000000000L) {
                        error = -EINVAL;
-                       goto out_free;
+                       goto out;
                }
                jiffies_left = timespec64_to_jiffies(timeout);
        }
 
+
        max = 0;
        for (sop = sops; sop < sops + nsops; sop++) {
                unsigned long mask = 1ULL << ((sop->sem_num) % BITS_PER_LONG);
@@ -2052,7 +2040,7 @@ static long do_semtimedop(int semid, struct sembuf __user *tsops,
                un = find_alloc_undo(ns, semid);
                if (IS_ERR(un)) {
                        error = PTR_ERR(un);
-                       goto out_free;
+                       goto out;
                }
        } else {
                un = NULL;
@@ -2063,25 +2051,25 @@ static long do_semtimedop(int semid, struct sembuf __user *tsops,
        if (IS_ERR(sma)) {
                rcu_read_unlock();
                error = PTR_ERR(sma);
-               goto out_free;
+               goto out;
        }
 
        error = -EFBIG;
        if (max >= sma->sem_nsems) {
                rcu_read_unlock();
-               goto out_free;
+               goto out;
        }
 
        error = -EACCES;
        if (ipcperms(ns, &sma->sem_perm, alter ? S_IWUGO : S_IRUGO)) {
                rcu_read_unlock();
-               goto out_free;
+               goto out;
        }
 
        error = security_sem_semop(&sma->sem_perm, sops, nsops, alter);
        if (error) {
                rcu_read_unlock();
-               goto out_free;
+               goto out;
        }
 
        error = -EIDRM;
@@ -2095,7 +2083,7 @@ static long do_semtimedop(int semid, struct sembuf __user *tsops,
         * entangled here and why it's RMID race safe on comments at sem_lock()
         */
        if (!ipc_valid_object(&sma->sem_perm))
-               goto out_unlock_free;
+               goto out_unlock;
        /*
         * semid identifiers are not unique - find_alloc_undo may have
         * allocated an undo structure, it was invalidated by an RMID
@@ -2104,7 +2092,7 @@ static long do_semtimedop(int semid, struct sembuf __user *tsops,
         * "un" itself is guaranteed by rcu.
         */
        if (un && un->semid == -1)
-               goto out_unlock_free;
+               goto out_unlock;
 
        queue.sops = sops;
        queue.nsops = nsops;
@@ -2130,10 +2118,10 @@ static long do_semtimedop(int semid, struct sembuf __user *tsops,
                rcu_read_unlock();
                wake_up_q(&wake_q);
 
-               goto out_free;
+               goto out;
        }
        if (error < 0) /* non-blocking error path */
-               goto out_unlock_free;
+               goto out_unlock;
 
        /*
         * We need to sleep on this operation, so we put the current
@@ -2198,14 +2186,14 @@ static long do_semtimedop(int semid, struct sembuf __user *tsops,
                if (error != -EINTR) {
                        /* see SEM_BARRIER_2 for purpose/pairing */
                        smp_acquire__after_ctrl_dep();
-                       goto out_free;
+                       goto out;
                }
 
                rcu_read_lock();
                locknum = sem_lock(sma, sops, nsops);
 
                if (!ipc_valid_object(&sma->sem_perm))
-                       goto out_unlock_free;
+                       goto out_unlock;
 
                /*
                 * No necessity for any barrier: We are protect by sem_lock()
@@ -2217,7 +2205,7 @@ static long do_semtimedop(int semid, struct sembuf __user *tsops,
                 * Leave without unlink_queue(), but with sem_unlock().
                 */
                if (error != -EINTR)
-                       goto out_unlock_free;
+                       goto out_unlock;
 
                /*
                 * If an interrupt occurred we have to clean up the queue.
@@ -2228,13 +2216,45 @@ static long do_semtimedop(int semid, struct sembuf __user *tsops,
 
        unlink_queue(sma, &queue);
 
-out_unlock_free:
+out_unlock:
        sem_unlock(sma, locknum);
        rcu_read_unlock();
+out:
+       return error;
+}
+
+static long do_semtimedop(int semid, struct sembuf __user *tsops,
+               unsigned nsops, const struct timespec64 *timeout)
+{
+       struct sembuf fast_sops[SEMOPM_FAST];
+       struct sembuf *sops = fast_sops;
+       struct ipc_namespace *ns;
+       int ret;
+
+       ns = current->nsproxy->ipc_ns;
+       if (nsops > ns->sc_semopm)
+               return -E2BIG;
+       if (nsops < 1)
+               return -EINVAL;
+
+       if (nsops > SEMOPM_FAST) {
+               sops = kvmalloc_array(nsops, sizeof(*sops), GFP_KERNEL);
+               if (sops == NULL)
+                       return -ENOMEM;
+       }
+
+       if (copy_from_user(sops, tsops, nsops * sizeof(*tsops))) {
+               ret =  -EFAULT;
+               goto out_free;
+       }
+
+       ret = __do_semtimedop(semid, sops, nsops, timeout, ns);
+
 out_free:
        if (sops != fast_sops)
                kvfree(sops);
-       return error;
+
+       return ret;
 }
 
 long ksys_semtimedop(int semid, struct sembuf __user *tsops,