xorl %eax, %eax

CVE-2009-4536: Linux kernel e1000 Integer Underflow

leave a comment »

This is another really cool issue disclosed by fabs on his 26c3 talk “cat /proc/sys/net/ipv4/fuckups“. He noticed that CVE-2009-1385 for which you can read my blog post here, was not fixing the bug at all.
The reason was that the e1000_clean_rx_irq() from drivers/net/e1000/e1000_main.c was incorrectly checking the initial frame’s length as you can see below:

 * e1000_clean_rx_irq - Send received data up the network stack; legacy
 * @adapter: board private structure
static bool e1000_clean_rx_irq(struct e1000_adapter *adapter,
                               struct e1000_rx_ring *rx_ring,
                               int *work_done, int work_to_do)
        u32 length;
                length = le16_to_cpu(rx_desc->length);
                /* !EOP means multiple descriptors were used to store a single
                 * packet, also make sure the frame isn't just CRC only */
                if (unlikely(!(status & E1000_RXD_STAT_EOP) || (length <= 4))) {
                        /* All receives must fit into a single buffer */
                        E1000_DBG("%s: Receive packet consumed multiple"
                                  " buffers\n", netdev->name);
                        /* recycle */
                        buffer_info->skb = skb;
                        goto next_desc;
                /* adjust length to remove Ethernet CRC, this must be
                 * done after the TBI_ACCEPT workaround above */
                length -= 4;

However, as fabs pointed out the bug was that part of the oversized frame would be spanning after the RX buffer and this partial frame would be later interpreted as an independent one. This was explained quite clear in the GIT commit like this:

This places us in a situation where, if we have a
spanning packet, the first part is discarded, but the second part is not (since
it is the end of packet, and it passes the EOP bit test).  If the second part of
the frame is small (4 bytes or less), we subtract 4 from it to remove its crc,
underflow the length

So, the above check that was added to fix this bug checks the size of the first frame and leaves the second part of it (which is stored in a different RX buffer because of its size) as it is. Now, if we have a look at drivers/net/e1000/e1000_hw.h we’ll see how the ‘e1000_rx_desc’ structure is defined:

/* Receive Descriptor */
struct e1000_rx_desc {
    __le64 buffer_addr; /* Address of the descriptor's data buffer */
    __le16 length;     /* Length of data DMAed into data buffer */
    __le16 csum;       /* Packet checksum */
    u8 status;      /* Descriptor status */
    u8 errors;      /* Descriptor Errors */
    __le16 special;

So, as fabs said, the above bugfix just checks that the last fragment is either the last packet or it has length (which you can see in the above structure) less than, or equal to four. Later on, the partial frame that is stored in a new RX buffer, will be interpreted as a valid, independent ethernet frame with no further checks. Definitely a really cool finding by fabs!

Written by xorl

January 2, 2010 at 03:10

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