xorl %eax, %eax

CVE-2009-4027: Linux kernel mac80211 Remote Kernel Panic

leave a comment »

I read about this issue from Eugene Teo’s email to oss-security mailing list. The bug was discovered by Johannes Berg and here you can see it as it was at net/mac80211/agg-rx.c of the 2.6.31 release of the Linux kernel…

void ieee80211_sta_stop_rx_ba_session(struct ieee80211_sub_if_data *sdata, u8 *ra, u16 tid,
                                        u16 initiator, u16 reason)
{
        struct ieee80211_local *local = sdata->local;
        struct sta_info *sta;

        /* stop HW Rx aggregation. ampdu_action existence
         * already verified in session init so we add the BUG_ON */
        BUG_ON(!local->ops->ampdu_action);

        rcu_read_lock();

        sta = sta_info_get(local, ra);
        if (!sta) {
                rcu_read_unlock();
                return;
        }

        __ieee80211_stop_rx_ba_session(sta, tid, initiator, reason);

        rcu_read_unlock();
}

The above function is called if the timer that is used for “AddBA” request expires. This routine retrieves the equivalent pointer from ‘sdata->local’ and performs a check using BUG_ON() to ensure that ‘local->ops->ampdu_action’ is not NULL. From include/net/mac80211.h we can read that this function pointer is used for:

* @ampdu_action: Perform a certain A-MPDU action
*      The RA/TID combination determines the destination and TID we want
*      the ampdu action to be performed for. The action is defined through
*      ieee80211_ampdu_mlme_action. Starting sequence number (@ssn)
*      is the first frame we expect to perform the action on. Notice
*      that TX/RX_STOP can pass NULL for this parameter.
*      Returns a negative error code on failure.

As Johannes Berg noticed, this callback could be set to NULL (and as we also read from the above comment) and thus, lead to the following code from include/asm-generic/bug.h:

/*
 * Don't use BUG() or BUG_ON() unless there's really no way out; one
 * example might be detecting data structure corruption in the middle
 * of an operation that can't be backed out of.  If the (sub)system
 * can somehow continue operating, perhaps with reduced functionality,
 * it's probably not BUG-worthy.
 *
 * If you're tempted to BUG(), think again:  is completely giving up
 * really the *only* solution?  There are usually better options, where
 * users don't need to reboot ASAP and can mostly shut down cleanly.
 */
#ifndef HAVE_ARCH_BUG
#define BUG() do { \
        printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); \
        panic("BUG!"); \
} while (0)
#endif

#ifndef HAVE_ARCH_BUG_ON
#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while(0)
#endif

Which is of course, a kernel panic that could be remotely triggered.

Off topic, trolling:
According to the OpenBSD developers this is just a reliability fix, hopefully, for the Linux people this is a remote vulnerability.

The patch was to remove this BUG_ON() call like this:

 	struct sta_info *sta;
 
-	/* stop HW Rx aggregation. ampdu_action existence
-	 * already verified in session init so we add the BUG_ON */
-	BUG_ON(!local->ops->ampdu_action);
-
 	rcu_read_lock();

The next bug, was also discovered and reported by Johannes Berg and you can find it under net/mac80211/agg-tx.c. This is definitely less important since the susceptible code is the following:

int ieee80211_stop_tx_ba_session(struct ieee80211_hw *hw,
                                 u8 *ra, u16 tid,
                                 enum ieee80211_back_parties initiator)
{
        struct ieee80211_local *local = hw_to_local(hw);
        struct sta_info *sta;
        int ret = 0;

        if (WARN_ON(!local->ops->ampdu_action))
                return -EINVAL;
  ...
}

Where the same check for ‘local->ops->ampdu_action’ callback takes place. Luckily, this is a WARN_ON() call that will only issue a printk() message because of this:

/*
 * WARN(), WARN_ON(), WARN_ON_ONCE, and so on can be used to report
 * significant issues that need prompt attention if they should ever
 * appear at runtime.  Use the versions with printk format strings
 * to provide better diagnostics.
 */
#ifndef __WARN
#ifndef __ASSEMBLY__
extern void warn_slowpath_fmt(const char *file, const int line,
                const char *fmt, ...) __attribute__((format(printf, 3, 4)));
extern void warn_slowpath_null(const char *file, const int line);
#define WANT_WARN_ON_SLOWPATH
#endif
#define __WARN()                warn_slowpath_null(__FILE__, __LINE__)
#define __WARN_printf(arg...)   warn_slowpath_fmt(__FILE__, __LINE__, arg)
#else
#define __WARN_printf(arg...)   do { printk(arg); __WARN(); } while (0)
#endif

#ifndef WARN_ON
#define WARN_ON(condition) ({                                           \
        int __ret_warn_on = !!(condition);                              \
        if (unlikely(__ret_warn_on))                                    \
                __WARN();                                               \
        unlikely(__ret_warn_on);                                        \
})
#endif

Of course, this ‘if’ clause was updated to remove that WARN_ON() call like this:

 	int ret = 0;
 
-	if (WARN_ON(!local->ops->ampdu_action))
+	if (!local->ops->ampdu_action)
 		return -EINVAL;
 
 	if (tid >= STA_TID_NUM)

About these ads

Written by xorl

December 2, 2009 at 16:37

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

Follow

Get every new post delivered to your Inbox.

Join 64 other followers