xorl %eax, %eax

CVE-2010-3084: Linux kernel ETHTOOL_GRXCLSRLALL Buffer Overflow

leave a comment »

A few days ago, Ben Hutchings reported a vulnerability along with a patch to the netdev mailing list. The vulnerability can be found at drivers/net/niu.c which part of the Neptune Ethernet device driver. Here is the buggy function:

static int niu_get_ethtool_tcam_all(struct niu *np,
                                    struct ethtool_rxnfc *nfc,
                                    u32 *rule_locs)
{
        struct niu_parent *parent = np->parent;
        struct niu_tcam_entry *tp;
        int i, idx, cnt;
        u16 n_entries;
        unsigned long flags;


        /* put the tcam size here */
        nfc->data = tcam_get_size(np);

        niu_lock_parent(np, flags);
        n_entries = nfc->rule_cnt;
        for (cnt = 0, i = 0; i < nfc->data; i++) {
                idx = tcam_get_index(np, i);
                tp = &parent->tcam[idx];
                if (!tp->valid)
                        continue;
                rule_locs[cnt] = i;
                cnt++;
        }
        niu_unlock_parent(np, flags);

        if (n_entries != cnt) {
                /* print warning, this should not happen */
                netdev_info(np->dev, "niu%d: In %s(): n_entries[%d] != cnt[%d]!!!\n",
                            np->parent->index, __func__, n_entries, cnt);
        }

        return 0;
}

The code is simple, ‘nfc->data’ (which is the NFC (Network Flow Classification) space for either RX flow hash value or rule DB size) is updated to contain the TCAM size using tcam_get_size() which is simply:

static u16 tcam_get_size(struct niu *np)
{
        /* One entry reserved for IP fragment rule */
        return np->clas.tcam_sz - 1;
}

Then, after locking the parent using a spinlock, it will initialize the ‘n_entries’ 16-bit long unsigned integer with the ‘nfc->rule_cnt’; this is the rules’ counter value. Next, it will enter a ‘for’ loop which uses ‘nfc->data’ as a limit (this contains the TCAM size) and it will just retrieve each index using tcam_get_index() and then use it inside ‘parent->tcam[]’ array in order to check the ‘valid’ Byte inside each TCAM entry. Finally, it will write to the ‘rule_locs[]’ array (which is passed as an argument to this function) the new indices of the defined rules.
When it exits the loop and releases the spinlock using niu_unlock_parent(), it will perform a final check which will print a warning using netdev_info() in case of ‘n_entries’ (containing rules’ counter value) is different from the newly updated counter value ‘cnt’.
The vulnerability here is that the function assumes that the output buffer (in this case ‘rule_locs[]’) has the correct size and as you’ve just read it will also issue a warning if the sizes are different. However, there is no actual check to ensure the ‘rule_locs[]’ array’s size. Furthermore, this array is user controlled through the following IOCTL command:

static int niu_get_nfc(struct net_device *dev, struct ethtool_rxnfc *cmd,
                       void *rule_locs)
{
        struct niu *np = netdev_priv(dev);
        int ret = 0;

        switch (cmd->cmd) {
       ...
        case ETHTOOL_GRXCLSRLALL:
                ret = niu_get_ethtool_tcam_all(np, cmd, (u32 *)rule_locs);
                break;
       ...
        return ret;
}

Which is also an unprivileged IOCTL command! To fix this bug the following patch was applied:

        int i, idx, cnt;
-       u16 n_entries;
        unsigned long flags;
-
+       int ret = 0;
 
        /* put the tcam size here */
        nfc->data = tcam_get_size(np);
 
        niu_lock_parent(np, flags);
-       n_entries = nfc->rule_cnt;
        for (cnt = 0, i = 0; i < nfc->data; i++) {
                idx = tcam_get_index(np, i);
                tp = &parent->tcam[idx];
                if (!tp->valid)
                        continue;
+               if (cnt == nfc->rule_cnt) {
+                       ret = -EMSGSIZE;
+                       break;
+               }
                rule_locs[cnt] = i;
                cnt++;
        }
        niu_unlock_parent(np, flags);
 
-       if (n_entries != cnt) {
-               /* print warning, this should not happen */
-               netdev_info(np->dev, "niu%d: In %s(): n_entries[%d] != cnt[%d]!!!\n",
-                           np->parent->index, __func__, n_entries, cnt);
-       }
-
-       return 0;
+       return ret;
 }

They removed the redundant ‘n_entries’ variable and added a new check inside the ‘for’ loop to break in case of reaching the ‘nfc->rule_cnt’ value with ‘EMSGSIZE’ (Error Message Too Long) return value. Finally, the warning message along with its check was also removed.
Regarding the exploitation, since the user controls the memory that will be overflowed he could use mmap(2) to make the memory corruption almost anywhere. I think it’s almost certainly exploitable but I haven’t tried it yet.

Written by xorl

September 22, 2010 at 19:17

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