xorl %eax, %eax

CVE-2007-6716: Linux kernel direct-io.c (dio) DoS

leave a comment »

This old vulnerability was reported by Joe Jin of Oracle on 26 July 2007. It affects Linux kernel up to 2.6.23 release and can be exploited by local users. DIO (Direct I/O) can be found under fs/drect-io.c on the Linux kernel source code tree. These routines are used to have direct I/O access to memory (what a surprise). Here is what could go wrong with it (code from 2.6.23)…

945 /*
946  * Releases both i_mutex and i_alloc_sem
947  */
948 static ssize_t
949 direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode, 
950        const struct iovec *iov, loff_t offset, unsigned long nr_segs, 
951        unsigned blkbits, get_block_t get_block, dio_iodone_t end_io,
952        struct dio *dio)
953 {
       ...
1012        
1013                ret = do_direct_IO(dio);
       ...
1019                if (ret) {
1020                        dio_cleanup(dio);
1021                        break;
1022                }
1023        } /* end iovec loop */
       ...
1101        return ret;
1102 }

What J. Jin noticed is that do_direct_IO() at line 1013 can return -EFAULT or -ENOMEM as well. If this is the case then the call to dio_cleanup() would be redundant since the dio structure passed to it would probably have members that are uninitialized and this will lead into passing “random” values to this:

369 static void dio_cleanup(struct dio *dio)
370 {
371        while (dio_pages_present(dio))
372                page_cache_release(dio_get_page(dio));
373 }

This goes deep inside memory management at mm/swap.c on functions like get_compound_page(), get_compound_page_dtor() which I’m not really familiar with in order to characterize this bug as exploitable. I gave the “DoS” title since this is how Linux kernel developers thought this vulnerability is. Anyway, passing “random” values to these functions results in a nice OOPS. Jin’s patch was to make direct_io_worker() checking every possible return address of do_direct_IO() like this:

-        if (ret) {
+        if (ret == -EFAULT || ret == -ENOMEM) 
+            goto out;
+        else if (ret) {
             dio_cleanup(dio);
             break;
         }
@@ -1113,6 +1115,7 @@ direct_io_worker(int rw, struct kiocb *i
     } else
         BUG_ON(ret != -EIOCBQUEUED);

+out:
     return ret;
 }

However, the patch which was finally merged was different. It was written by Zach Brown and it removed the redundant initialization of various dio structure members and added a simple kzalloc() instead of kmalloc() and this way it zeros out the contents of the whole dio structure and consequently initializes it. Here is the final patch:

@@ -958,36 +958,22 @@ direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode,
        ssize_t ret2;
        size_t bytes;

-       dio->bio = NULL;
        dio->inode = inode;
        dio->rw = rw;
        dio->blkbits = blkbits;
        dio->blkfactor = inode->i_blkbits - blkbits;
-       dio->start_zero_done = 0;
-       dio->size = 0;
        dio->block_in_file = offset >> blkbits;
-       dio->blocks_available = 0;
-       dio->cur_page = NULL; 
-       dio->boundary = 0;
-       dio->reap_counter = 0;
        dio->get_block = get_block;
        dio->end_io = end_io;
-       dio->map_bh.b_private = NULL;
-       dio->map_bh.b_state = 0;
        dio->final_block_in_bio = -1;
        dio->next_block_for_io = -1;

-       dio->page_errors = 0;
-       dio->io_error = 0;
-       dio->result = 0;
        dio->iocb = iocb;
        dio->i_size = i_size_read(inode);

        spin_lock_init(&dio->bio_lock);
        dio->refcount = 1;
-       dio->bio_list = NULL;
-       dio->waiter = NULL;

        /*
         * In case of non-aligned buffers, we may need 2 more
@@ -995,8 +981,6 @@ direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode,
         */
        if (unlikely(dio->blkfactor))
                dio->pages_in_io = 2;
-       else
-               dio->pages_in_io = 0;

        for (seg = 0; seg < nr_segs; seg++) {
                user_addr = (unsigned long)iov[seg].iov_base;
@@ -1184,7 +1168,7 @@ __blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
                }
        }

-       dio = kmalloc(sizeof(*dio), GFP_KERNEL);
+       dio = kzalloc(sizeof(*dio), GFP_KERNEL);
        retval = -ENOMEM;
        if (!dio)
                goto out;

Again, I assume this bug is a DoS because of my lack of knowledge of the internal memory management routines being used in the above code. Passing a “random” value usually means that you can have something more than just a crash.

Written by xorl

March 27, 2009 at 17:14

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