Comment 10 for bug 337929

Revision history for this message
Andy Whitcroft (apw) wrote :

We are seeing panics coming out of wiphy_update_regulatory() from LBM on
both Jaunty and Intrepid. These two carry the same basic code for the
wireless stack. Sample stack trace as here:

    [ 398.693189] BUG: unable to handle kernel NULL pointer dereference at 00000004
    [ 398.693206] IP: [<f8e1feb4>] :lbm_cw_cfg80211:wiphy_update_regulatory+0x1d4/0x420
    [ 398.693232] *pde = 00000000
    [ 398.693242] Oops: 0000 [#1] SMP
[...]
    [ 398.693479]
    [ 398.693487] Pid: 6314, comm: modprobe Not tainted (2.6.27-14-generic #1)
    [ 398.693496] EIP: 0060:[<f8e1feb4>] EFLAGS: 00010246 CPU: 1
    [ 398.693514] EIP is at wiphy_update_regulatory+0x1d4/0x420 [lbm_cw_cfg80211]
    [ 398.693523] EAX: 00000000 EBX: 00000410 ECX: f52e0060 EDX: 00000001
    [ 398.693532] ESI: f52e0000 EDI: f52e0060 EBP: f520fcb0 ESP: f520fc78
    [ 398.693541] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
    [ 398.693550] Process modprobe (pid: 6314, ti=f520e000 task=f5250000 task.ti=f520e000)
    [ 398.693558] Stack: c018a2ee c0495500 f52e0060 f6603000 f520fcc8 c01050f8 00004e17 013ef000
    [ 398.693581] 00000099 f8e1eece f52e0060 00000410 f52e0000 f52e0060 f520fccc f8e1e52e
    [ 398.693603] 00000002 00000002 fffffff4 00000000 f52e01e0 f520fd00 f8f472f0 00006957

The key thing to note is that we are panicing with a data reference to address 0x4. This is classically a structure offset relative to a NULL pointer. Looking at this function when we are called we may look at the band, and then we will handle beacons, and finally we will give the PHY a chance to see the information via the reg_notifier. None of the referenced information is at offset 0x4 in the structure:

    void wiphy_update_regulatory(struct wiphy *wiphy,
     enum nl80211_reg_initiator initiator)
    {
     enum ieee80211_band band;

     if (ignore_reg_update(wiphy, initiator))
      goto out;
     for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
      if (wiphy->bands[band])
       handle_band(wiphy, band);
     }
    out:
     reg_process_beacons(wiphy);
     if (wiphy->reg_notifier)
      wiphy->reg_notifier(wiphy, last_request);
    }

Looking at the reg_process_beacons() we can see it cals: reg_is_world_romaing():

    /* Reap the advantages of previously found beacons */
    static void reg_process_beacons(struct wiphy *wiphy)
    {
     if (!reg_is_world_roaming(wiphy))
      return;
     wiphy_update_beacon_reg(wiphy);
    }

and that references last_request->initiator:

    static bool reg_is_world_roaming(struct wiphy *wiphy)
    {
     if (is_world_regdom(cfg80211_regdomain->alpha2) ||
  (wiphy->regd && is_world_regdom(wiphy->regd->alpha2)))
      return true;
     if (last_request->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE &&
  wiphy->custom_regulatory)
      return true;
     return false;
    }

and _that_ is at offset 4, and as these two are static they are likely inlined into wiphy_update_regulatory():

    struct regulatory_request {
     int wiphy_idx;
     enum nl80211_reg_initiator initiator;
    [...]
    };

Note also that the definition of ignore_reg_updates() does the following:

    static bool ignore_reg_update(struct wiphy *wiphy,
      enum nl80211_reg_initiator initiator)
    {
     if (!last_request)
      return true;
    [...]
    }

So, it is entirly possible to call wiphy_update_regulatory() with last_request == NULL, and for that to trigger us to call down to reg_process_beacons() to reg_process_beacons() to reg_is_world_roaming() which then reference last_request assuming it is not NULL leading to a panic referencing 0x4.

As a side note we will also call any notifiers with las_request as NULL and cirtainly the ath9k notifier is not expecting that to be so which would likely also blammo in the same manner:

    int ath9k_reg_notifier(struct wiphy *wiphy, struct regulatory_request *request)
    {
    [...]
     switch (request->initiator) {
    [...]
    }

It seems likely we should not perform any of this processing is last_request is null. Will patch this up for testing.