xorl %eax, %eax

CVE-2008-3911: Linux kernel SunRPC Read Out of Bounds

leave a comment »

This was initially reported by Vegard Nossum who also provided a reproduce code on 30 August 2008. This affects 2.6.26.3 and probably earlier releases of the Linux kernel. It was patched by Cyrill Gorcunov on 31 August 2008. Here is the vulnerable function:

59 static int proc_do_xprt(ctl_table *table, int write, struct file *file,
60                        void __user *buffer, size_t *lenp, loff_t *ppos)
61 {
62        char tmpbuf[256];
63        int len;
64        if ((*ppos && !write) || !*lenp) {
65                *lenp = 0;
66                return 0;
67        }
68        if (write)
69                return -EINVAL;
70        else {
71                len = svc_print_xprts(tmpbuf, sizeof(tmpbuf));
72                if (!access_ok(VERIFY_WRITE, buffer, len))
73                        return -EFAULT;
74
75                if (__copy_to_user(buffer, tmpbuf, len))
76                        return -EFAULT;
77        }
78        *lenp -= len;
79        *ppos += len;
80        return 0;
81 }

This can be found at net/sunrpc/sysctl.c which is the SysCtl interface for the SunRPC module of the Linux kernel. The bug is extremely easy to spot. If user attempts to write (line 68) to /proc/sys/sunrpc/transports it immediately returns with INVALID error code. In any other case, it uses svc_print_xprts() to format the transport list for printing. And finally, a usual structure of checking the memory area using access_ok() and copying data from tmpbuf to buffer using copy_to_user(). This is the user space buffer with the size lenp provided by the user. The first proposed patch was to simply check whether the requested length (lenp) is less than tmpbuf‘s length len like this:

         len = svc_print_xprts(tmpbuf, sizeof(tmpbuf));
+        if (*lenp < len)
+            return -EFAULT;
         if (!access_ok(VERIFY_WRITE, buffer, len))

However, the final patch to this buffer overrun condition was this:

        char tmpbuf[256];
-       int len;
+       size_t len;
+
        if ((*ppos && !write) || !*lenp) {
                *lenp = 0;
                return 0;
        }
-       if (write)
-               return -EINVAL;
-       else {
-               len = svc_print_xprts(tmpbuf, sizeof(tmpbuf));
-               if (!access_ok(VERIFY_WRITE, buffer, len))
-                       return -EFAULT;
-
-               if (__copy_to_user(buffer, tmpbuf, len))
-                       return -EFAULT;
-       }
-       *lenp -= len;
-       *ppos += len;
-       return 0;
+       len = svc_print_xprts(tmpbuf, sizeof(tmpbuf));
+       return simple_read_from_buffer(buffer, *lenp, ppos, tmpbuf, len);
}

First of all, congratulations for being one of the few Linux kernel developers who use size_t for length variables. Now, as you can see it completely removes the write clause and now it only contains two calls. One to format the tmpbuf as expected and one to simple_read_from_buffer() which is a commonly used routine from fs/libfs.c that does this:

515/**
516 * simple_read_from_buffer - copy data from the buffer to user space
517 * @to: the user space buffer to read to
518 * @count: the maximum number of bytes to read
519 * @ppos: the current position in the buffer
520 * @from: the buffer to read from
521 * @available: the size of the buffer
522 *
523 * The simple_read_from_buffer() function reads up to @count bytes from the
524 * buffer @from at offset @ppos into the user space address starting at @to.
525 *
526 * On success, the number of bytes read is returned and the offset @ppos is
527 * advanced by this number, or negative value is returned on error.
528 **/
529 ssize_t simple_read_from_buffer(void __user *to, size_t count, loff_t *ppos,
530                                 const void *from, size_t available)
531 {
532        loff_t pos = *ppos;
533        if (pos < 0)
534                return -EINVAL;
535        if (pos >= available)
536                return 0;
537        if (count > available - pos)
538                count = available - pos;
539        if (copy_to_user(to, from + pos, count))
540                return -EFAULT;
541        *ppos = pos + count;
542        return count;
543 }


It checks for the requested and the available sizes at line 535 and this way you eliminate any ugly hacks like the one at the initial patch.

Written by xorl

April 7, 2009 at 13:33

Posted in bugs, linux

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s