xorl %eax, %eax

CVE-2009-3288: Linux kernel sg_build_indirect() Double Free

leave a comment »

This bug affects Linux kernel releases 2.6.28-rc1 through 2.6.31-rc8 and it was reported to the LKML by Bob Tracy on 2 September 2009. The bug can be found at drivers/scsi/sg.c like this…

static int
sg_build_indirect(Sg_scatter_hold * schp, Sg_fd * sfp, int buff_size)
{
        int ret_sz = 0, i, k, rem_sz, num, mx_sc_elems;
        int sg_tablesize = sfp->parentdp->sg_tablesize;
        int blk_size = buff_size, order;
        gfp_t gfp_mask = GFP_ATOMIC | __GFP_COMP | __GFP_NOWARN;
    ...
        for (k = 0, rem_sz = blk_size; rem_sz > 0 && k < mx_sc_elems;
             k++, rem_sz -= ret_sz) {
    ...
                schp->pages[k] = alloc_pages(gfp_mask, order);
                if (!schp->pages[k])
                        goto out;
    ...
out:
        for (i = 0; i < k; i++)
                __free_pages(schp->pages[k], order);

        if (--order >= 0)
                goto retry;

        return -ENOMEM;
}

The problem with this code is that the allocation for loop uses ‘k’ as the counter which shouldn’t be more than ‘mx_sc_elems’ and if everything is fine, it will allocate the equivalent ‘schp->pages[k]’ pointer using alloc_pages(). However, if it fails it will jump to ‘out’ label that is using __free_pages() in another for loop to free the allocated space. As you can see, it incorrectly uses ‘k’ as an index even though the for loop uses ‘i’ as a counter. This means that the first iteration will almost certainly succeed, but the next one will attempt to free the exact same space.
The fix, was of course to use the right counter in the deallocation loop like this:

 	for (i = 0; i < k; i++)
-		__free_pages(schp->pages[k], order);
+		__free_pages(schp->pages[i], order);
 
 	if (--order >= 0)

Bob Tracy was able to trigger that bug through “xcdroast” as we can read from his report.

Written by xorl

October 16, 2009 at 21:24

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