xorl %eax, %eax

Linux kernel wl1271 test-mode NVS Heap Memory Corruption

with 2 comments

This bug was reported by Ido Yariv and affects Linux kernel from 2.6.36 up to 2.6.38.3 (not including). So, the susceptible routine is part of drivers/net/wireless/wl12xx/testmode.c file.

enum wl1271_tm_commands {
  ...
        WL1271_TM_CMD_NVS_PUSH,
  ...
};
  ...
int wl1271_tm_cmd(struct ieee80211_hw *hw, void *data, int len)
{
        struct wl1271 *wl = hw->priv;
        struct nlattr *tb[WL1271_TM_ATTR_MAX + 1];
        int err;

        err = nla_parse(tb, WL1271_TM_ATTR_MAX, data, len, wl1271_tm_policy);
  ...
        switch (nla_get_u32(tb[WL1271_TM_ATTR_CMD_ID])) {
  ...
        case WL1271_TM_CMD_NVS_PUSH:
                return wl1271_tm_cmd_nvs_push(wl, tb);
  ...
        default:
                return -EOPNOTSUPP;
        }
}

Which transfers the code execution to wl1271_tm_cmd_nvs_push() passing the NetLink derived attributes.

static int wl1271_tm_cmd_nvs_push(struct wl1271 *wl, struct nlattr *tb[])
{
        int ret = 0;
        size_t len;
        void *buf;

        wl1271_debug(DEBUG_TESTMODE, "testmode cmd nvs push");

        if (!tb[WL1271_TM_ATTR_DATA])
                return -EINVAL;

        buf = nla_data(tb[WL1271_TM_ATTR_DATA]);
        len = nla_len(tb[WL1271_TM_ATTR_DATA]);

        mutex_lock(&wl->mutex);

        kfree(wl->nvs);

        wl->nvs = kzalloc(sizeof(struct wl1271_nvs_file), GFP_KERNEL);
  ...
        memcpy(wl->nvs, buf, len);
        wl->nvs_len = len;

        wl1271_debug(DEBUG_TESTMODE, "testmode pushed nvs");

out:
        mutex_unlock(&wl->mutex);

        return ret;
}

The issue here is quite simple. The ‘wl->nvs’ pointer will always be initialized to a heap area with size of ‘sizeof(struct wl1271_nvs_file)’ but on the other hand, the subsequent memcpy() routine will copy ‘len’ Bytes. By passing a ‘len’ greater than the size of ‘wl->nvs’ a kernel heap memory corruption will occur.

The fix was to add the missing checks.

 
-	wl->nvs = kzalloc(sizeof(struct wl1271_nvs_file), GFP_KERNEL);
+	if (len != sizeof(struct wl1271_nvs_file))
+		return -EINVAL;
+
+	wl->nvs = kzalloc(len, GFP_KERNEL);
 	if (!wl->nvs) {

Written by xorl

June 9, 2011 at 13:52

Posted in bugs, linux

2 Responses

Subscribe to comments with RSS.

  1. isn’t the added check 2 statements too low?

    in case of size of size mismatch wl->mutex will never be unlocked and wl->nvs will be left as dangling pointer.

    don_jones

    June 10, 2011 at 22:57

  2. From the given snippet it seems you’re right.

    xorl

    June 11, 2011 at 15:21


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