sys: don't hold uts_sem while accessing userspace memory

commit 42a0cc3478584d4d63f68f2f5af021ddbea771fa upstream.

Holding uts_sem as a writer while accessing userspace memory allows a
namespace admin to stall all processes that attempt to take uts_sem.
Instead, move data through stack buffers and don't access userspace memory
while uts_sem is held.

Cc: stable@vger.kernel.org
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c
index e0903215982d897563aaaed15eef07bed01fa943..bbc7cb9faa01ce585215bad9b5a46e3a75379403 100644 (file)
--- a/arch/alpha/kernel/osf_sys.c
+++ b/arch/alpha/kernel/osf_sys.c
@@ -526,24 +526,19 @@ SYSCALL_DEFINE4(osf_mount, unsigned long, typenr, const char __user *, path,
 SYSCALL_DEFINE1(osf_utsname, char __user *, name)
 {
        int error;
+       char tmp[5 * 32];
 
        down_read(&uts_sem);
-       error = -EFAULT;
-       if (copy_to_user(name + 0, utsname()->sysname, 32))
-               goto out;
-       if (copy_to_user(name + 32, utsname()->nodename, 32))
-               goto out;
-       if (copy_to_user(name + 64, utsname()->release, 32))
-               goto out;
-       if (copy_to_user(name + 96, utsname()->version, 32))
-               goto out;
-       if (copy_to_user(name + 128, utsname()->machine, 32))
-               goto out;
+       memcpy(tmp + 0 * 32, utsname()->sysname, 32);
+       memcpy(tmp + 1 * 32, utsname()->nodename, 32);
+       memcpy(tmp + 2 * 32, utsname()->release, 32);
+       memcpy(tmp + 3 * 32, utsname()->version, 32);
+       memcpy(tmp + 4 * 32, utsname()->machine, 32);
+       up_read(&uts_sem);
 
-       error = 0;
- out:
-       up_read(&uts_sem);      
-       return error;
+       if (copy_to_user(name, tmp, sizeof(tmp)))
+               return -EFAULT;
+       return 0;
 }
 
 SYSCALL_DEFINE0(getpagesize)
@@ -563,18 +558,21 @@ SYSCALL_DEFINE2(osf_getdomainname, char __user *, name, int, namelen)
 {
        int len, err = 0;
        char *kname;
+       char tmp[32];
 
-       if (namelen > 32)
+       if (namelen < 0 || namelen > 32)
                namelen = 32;
 
        down_read(&uts_sem);
        kname = utsname()->domainname;
        len = strnlen(kname, namelen);
-       if (copy_to_user(name, kname, min(len + 1, namelen)))
-               err = -EFAULT;
+       len = min(len + 1, namelen);
+       memcpy(tmp, kname, len);
        up_read(&uts_sem);
 
-       return err;
+       if (copy_to_user(name, tmp, len))
+               return -EFAULT;
+       return 0;
 }
 
 /*
@@ -736,13 +734,14 @@ SYSCALL_DEFINE3(osf_sysinfo, int, command, char __user *, buf, long, count)
        };
        unsigned long offset;
        const char *res;
-       long len, err = -EINVAL;
+       long len;
+       char tmp[__NEW_UTS_LEN + 1];
 
        offset = command-1;
        if (offset >= ARRAY_SIZE(sysinfo_table)) {
                /* Digital UNIX has a few unpublished interfaces here */
                printk("sysinfo(%d)", command);
-               goto out;
+               return -EINVAL;
        }
 
        down_read(&uts_sem);
@@ -750,13 +749,11 @@ SYSCALL_DEFINE3(osf_sysinfo, int, command, char __user *, buf, long, count)
        len = strlen(res)+1;
        if ((unsigned long)len > (unsigned long)count)
                len = count;
-       if (copy_to_user(buf, res, len))
-               err = -EFAULT;
-       else
-               err = 0;
+       memcpy(tmp, res, len);
        up_read(&uts_sem);
- out:
-       return err;
+       if (copy_to_user(buf, tmp, len))
+               return -EFAULT;
+       return 0;
 }
 
 SYSCALL_DEFINE5(osf_getsysinfo, unsigned long, op, void __user *, buffer,

diff --git a/arch/sparc/kernel/sys_sparc_32.c b/arch/sparc/kernel/sys_sparc_32.c
index 646988d4c1a35aca56c0a3f0c56abdf1ca183587..740f43b9b54192a7a202a980a94475bf3445482b 100644 (file)
--- a/arch/sparc/kernel/sys_sparc_32.c
+++ b/arch/sparc/kernel/sys_sparc_32.c
@@ -201,23 +201,27 @@ SYSCALL_DEFINE5(rt_sigaction, int, sig,
 
 asmlinkage long sys_getdomainname(char __user *name, int len)
 {
-       int nlen, err;
-       
+       int nlen, err;
+       char tmp[__NEW_UTS_LEN + 1];
+
        if (len < 0)
                return -EINVAL;
 
-       down_read(&uts_sem);
-       
+       down_read(&uts_sem);
+
        nlen = strlen(utsname()->domainname) + 1;
        err = -EINVAL;
        if (nlen > len)
-               goto out;
+               goto out_unlock;
+       memcpy(tmp, utsname()->domainname, nlen);
 
-       err = -EFAULT;
-       if (!copy_to_user(name, utsname()->domainname, nlen))
-               err = 0;
+       up_read(&uts_sem);
 
-out:
+       if (copy_to_user(name, tmp, nlen))
+               return -EFAULT;
+       return 0;
+
+out_unlock:
        up_read(&uts_sem);
        return err;
 }

diff --git a/arch/sparc/kernel/sys_sparc_64.c b/arch/sparc/kernel/sys_sparc_64.c
index 7f0f7c01b29717878968ef6804df96fbe800f586..f63cd2ea8470671c3569c7d1306f107f3f395b1f 100644 (file)
--- a/arch/sparc/kernel/sys_sparc_64.c
+++ b/arch/sparc/kernel/sys_sparc_64.c
@@ -524,23 +524,27 @@ extern void check_pending(int signum);
 
 SYSCALL_DEFINE2(getdomainname, char __user *, name, int, len)
 {
-        int nlen, err;
+       int nlen, err;
+       char tmp[__NEW_UTS_LEN + 1];
 
        if (len < 0)
                return -EINVAL;
 
-       down_read(&uts_sem);
-       
+       down_read(&uts_sem);
+
        nlen = strlen(utsname()->domainname) + 1;
        err = -EINVAL;
        if (nlen > len)
-               goto out;
+               goto out_unlock;
+       memcpy(tmp, utsname()->domainname, nlen);
+
+       up_read(&uts_sem);
 
-       err = -EFAULT;
-       if (!copy_to_user(name, utsname()->domainname, nlen))
-               err = 0;
+       if (copy_to_user(name, tmp, nlen))
+               return -EFAULT;
+       return 0;
 
-out:
+out_unlock:
        up_read(&uts_sem);
        return err;
 }

diff --git a/kernel/sys.c b/kernel/sys.c
index f718742e55e6fc4e746a3c340ebc3bfa3ff64e6e..e2446ade79ba700dd427b98f26af30ca67090fa2 100644 (file)
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1142,18 +1142,19 @@ static int override_release(char __user *release, size_t len)
 
 SYSCALL_DEFINE1(newuname, struct new_utsname __user *, name)
 {
-       int errno = 0;
+       struct new_utsname tmp;
 
        down_read(&uts_sem);
-       if (copy_to_user(name, utsname(), sizeof *name))
-               errno = -EFAULT;
+       memcpy(&tmp, utsname(), sizeof(tmp));
        up_read(&uts_sem);
+       if (copy_to_user(name, &tmp, sizeof(tmp)))
+               return -EFAULT;
 
-       if (!errno && override_release(name->release, sizeof(name->release)))
-               errno = -EFAULT;
-       if (!errno && override_architecture(name))
-               errno = -EFAULT;
-       return errno;
+       if (override_release(name->release, sizeof(name->release)))
+               return -EFAULT;
+       if (override_architecture(name))
+               return -EFAULT;
+       return 0;
 }
 
 #ifdef __ARCH_WANT_SYS_OLD_UNAME
@@ -1162,55 +1163,46 @@ SYSCALL_DEFINE1(newuname, struct new_utsname __user *, name)
  */
 SYSCALL_DEFINE1(uname, struct old_utsname __user *, name)
 {
-       int error = 0;
+       struct old_utsname tmp;
 
        if (!name)
                return -EFAULT;
 
        down_read(&uts_sem);
-       if (copy_to_user(name, utsname(), sizeof(*name)))
-               error = -EFAULT;
+       memcpy(&tmp, utsname(), sizeof(tmp));
        up_read(&uts_sem);
+       if (copy_to_user(name, &tmp, sizeof(tmp)))
+               return -EFAULT;
 
-       if (!error && override_release(name->release, sizeof(name->release)))
-               error = -EFAULT;
-       if (!error && override_architecture(name))
-               error = -EFAULT;
-       return error;
+       if (override_release(name->release, sizeof(name->release)))
+               return -EFAULT;
+       if (override_architecture(name))
+               return -EFAULT;
+       return 0;
 }
 
 SYSCALL_DEFINE1(olduname, struct oldold_utsname __user *, name)
 {
-       int error;
+       struct oldold_utsname tmp = {};
 
        if (!name)
                return -EFAULT;
-       if (!access_ok(VERIFY_WRITE, name, sizeof(struct oldold_utsname)))
-               return -EFAULT;
 
        down_read(&uts_sem);
-       error = __copy_to_user(&name->sysname, &utsname()->sysname,
-                              __OLD_UTS_LEN);
-       error |= __put_user(0, name->sysname + __OLD_UTS_LEN);
-       error |= __copy_to_user(&name->nodename, &utsname()->nodename,
-                               __OLD_UTS_LEN);
-       error |= __put_user(0, name->nodename + __OLD_UTS_LEN);
-       error |= __copy_to_user(&name->release, &utsname()->release,
-                               __OLD_UTS_LEN);
-       error |= __put_user(0, name->release + __OLD_UTS_LEN);
-       error |= __copy_to_user(&name->version, &utsname()->version,
-                               __OLD_UTS_LEN);
-       error |= __put_user(0, name->version + __OLD_UTS_LEN);
-       error |= __copy_to_user(&name->machine, &utsname()->machine,
-                               __OLD_UTS_LEN);
-       error |= __put_user(0, name->machine + __OLD_UTS_LEN);
+       memcpy(&tmp.sysname, &utsname()->sysname, __OLD_UTS_LEN);
+       memcpy(&tmp.nodename, &utsname()->nodename, __OLD_UTS_LEN);
+       memcpy(&tmp.release, &utsname()->release, __OLD_UTS_LEN);
+       memcpy(&tmp.version, &utsname()->version, __OLD_UTS_LEN);
+       memcpy(&tmp.machine, &utsname()->machine, __OLD_UTS_LEN);
        up_read(&uts_sem);
+       if (copy_to_user(name, &tmp, sizeof(tmp)))
+               return -EFAULT;
 
-       if (!error && override_architecture(name))
-               error = -EFAULT;
-       if (!error && override_release(name->release, sizeof(name->release)))
-               error = -EFAULT;
-       return error ? -EFAULT : 0;
+       if (override_architecture(name))
+               return -EFAULT;
+       if (override_release(name->release, sizeof(name->release)))
+               return -EFAULT;
+       return 0;
 }
 #endif
 
@@ -1224,17 +1216,18 @@ SYSCALL_DEFINE2(sethostname, char __user *, name, int, len)
 
        if (len < 0 || len > __NEW_UTS_LEN)
                return -EINVAL;
-       down_write(&uts_sem);
        errno = -EFAULT;
        if (!copy_from_user(tmp, name, len)) {
-               struct new_utsname *u = utsname();
+               struct new_utsname *u;
 
+               down_write(&uts_sem);
+               u = utsname();
                memcpy(u->nodename, tmp, len);
                memset(u->nodename + len, 0, sizeof(u->nodename) - len);
                errno = 0;
                uts_proc_notify(UTS_PROC_HOSTNAME);
+               up_write(&uts_sem);
        }
-       up_write(&uts_sem);
        return errno;
 }
 
@@ -1242,8 +1235,9 @@ SYSCALL_DEFINE2(sethostname, char __user *, name, int, len)
 
 SYSCALL_DEFINE2(gethostname, char __user *, name, int, len)
 {
-       int i, errno;
+       int i;
        struct new_utsname *u;
+       char tmp[__NEW_UTS_LEN + 1];
 
        if (len < 0)
                return -EINVAL;
@@ -1252,11 +1246,11 @@ SYSCALL_DEFINE2(gethostname, char __user *, name, int, len)
        i = 1 + strlen(u->nodename);
        if (i > len)
                i = len;
-       errno = 0;
-       if (copy_to_user(name, u->nodename, i))
-               errno = -EFAULT;
+       memcpy(tmp, u->nodename, i);
        up_read(&uts_sem);
-       return errno;
+       if (copy_to_user(name, tmp, i))
+               return -EFAULT;
+       return 0;
 }
 
 #endif
@@ -1275,17 +1269,18 @@ SYSCALL_DEFINE2(setdomainname, char __user *, name, int, len)
        if (len < 0 || len > __NEW_UTS_LEN)
                return -EINVAL;
 
-       down_write(&uts_sem);
        errno = -EFAULT;
        if (!copy_from_user(tmp, name, len)) {
-               struct new_utsname *u = utsname();
+               struct new_utsname *u;
 
+               down_write(&uts_sem);
+               u = utsname();
                memcpy(u->domainname, tmp, len);
                memset(u->domainname + len, 0, sizeof(u->domainname) - len);
                errno = 0;
                uts_proc_notify(UTS_PROC_DOMAINNAME);
+               up_write(&uts_sem);
        }
-       up_write(&uts_sem);
        return errno;
 }
 

diff --git a/kernel/utsname_sysctl.c b/kernel/utsname_sysctl.c
index c8eac43267e90d13aee860f9a10eb89282e408f2..d2b3b2973456e82a91635f718f98e41157b510ee 100644 (file)
--- a/kernel/utsname_sysctl.c
+++ b/kernel/utsname_sysctl.c
@@ -17,7 +17,7 @@
 
 #ifdef CONFIG_PROC_SYSCTL
 
-static void *get_uts(struct ctl_table *table, int write)
+static void *get_uts(struct ctl_table *table)
 {
        char *which = table->data;
        struct uts_namespace *uts_ns;
@@ -25,21 +25,9 @@ static void *get_uts(struct ctl_table *table, int write)
        uts_ns = current->nsproxy->uts_ns;
        which = (which - (char *)&init_uts_ns) + (char *)uts_ns;
 
-       if (!write)
-               down_read(&uts_sem);
-       else
-               down_write(&uts_sem);
        return which;
 }
 
-static void put_uts(struct ctl_table *table, int write, void *which)
-{
-       if (!write)
-               up_read(&uts_sem);
-       else
-               up_write(&uts_sem);
-}
-
 /*
  *     Special case of dostring for the UTS structure. This has locks
  *     to observe. Should this be in kernel/sys.c ????
@@ -49,13 +37,34 @@ static int proc_do_uts_string(struct ctl_table *table, int write,
 {
        struct ctl_table uts_table;
        int r;
+       char tmp_data[__NEW_UTS_LEN + 1];
+
        memcpy(&uts_table, table, sizeof(uts_table));
-       uts_table.data = get_uts(table, write);
+       uts_table.data = tmp_data;
+
+       /*
+        * Buffer the value in tmp_data so that proc_dostring() can be called
+        * without holding any locks.
+        * We also need to read the original value in the write==1 case to
+        * support partial writes.
+        */
+       down_read(&uts_sem);
+       memcpy(tmp_data, get_uts(table), sizeof(tmp_data));
+       up_read(&uts_sem);
        r = proc_dostring(&uts_table, write, buffer, lenp, ppos);
-       put_uts(table, write, uts_table.data);
 
-       if (write)
+       if (write) {
+               /*
+                * Write back the new value.
+                * Note that, since we dropped uts_sem, the result can
+                * theoretically be incorrect if there are two parallel writes
+                * at non-zero offsets to the same sysctl.
+                */
+               down_write(&uts_sem);
+               memcpy(get_uts(table), tmp_data, sizeof(tmp_data));
+               up_write(&uts_sem);
                proc_sys_poll_notify(table->poll);
+       }
 
        return r;
 }