Search Linux Wireless

Re: [RFC v2] mac80211: fix possible null-pointer dereference

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Johannes

On Fri, Oct 8, 2010 at 11:03 AM, Johannes Berg
<johannes@xxxxxxxxxxxxxxxx> wrote:
>> The patch not only solves this problem, but also responds correctly to
>> non-compatible PLINK_OPEN frames by generating a PLINK_CLOSE with the
>> right reason code.
>
> But then you can't ever actually properly process a *matching*
> PLINK_OPEN frame, afaict, because those definitely do have
>        "!sta && ftype == PLINK_OPEN"

Not always: an sta can be created on reception of a beacon or probe
response (mesh_neighbour_update()), so you could have ftype ==
PLINK_OPEN and a non-null sta. Buy, yes, I agree that !sta && ftype ==
PLINK_OPEN is a valid case.

> which is how the "if (!sta && ftype != PLINK_OPEN) return" came about,
> afaict.

Yes, and Christian's *second* patch (submitted as RFC on Sept 24) does
not change that check.  The matching PLINK_OPEN would be handled in
the following branch:

        } else if (!sta) {
                /* ftype == PLINK_OPEN */
                u32 rates;
                (...)
                sta = mesh_plink_alloc(sdata, mgmt->sa, rates);

> So I still don't think it's quite correct to fix it this way.

Could it be that we are looking at different patches?  Just to be
sure, let me paste below the one we are referring to as sent by
Christian:

---
diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
index ea13a80..8b7efee 100644
--- a/net/mac80211/mesh_plink.c
+++ b/net/mac80211/mesh_plink.c
@@ -412,7 +412,7 @@ void mesh_rx_plink_frame(struct
ieee80211_sub_if_data *sdata, struct ieee80211_m
       enum plink_event event;
       enum plink_frame_type ftype;
       size_t baselen;
-       bool deactivated;
+       bool deactivated, matches_local = true;
       u8 ie_len;
       u8 *baseaddr;
       __le16 plid, llid, reason;
@@ -487,6 +487,7 @@ void mesh_rx_plink_frame(struct
ieee80211_sub_if_data *sdata, struct ieee80211_m
       /* Now we will figure out the appropriate event... */
       event = PLINK_UNDEFINED;
       if (ftype != PLINK_CLOSE && (!mesh_matches_local(&elems, sdata))) {
+               matches_local = false;
               switch (ftype) {
               case PLINK_OPEN:
                       event = OPN_RJCT;
@@ -498,7 +499,15 @@ void mesh_rx_plink_frame(struct
ieee80211_sub_if_data *sdata, struct ieee80211_m
                       /* avoid warning */
                       break;
               }
-               spin_lock_bh(&sta->lock);
+       }
+
+       if (!sta && !matches_local) {
+               rcu_read_unlock();
+               reason = cpu_to_le16(MESH_CAPABILITY_POLICY_VIOLATION);
+               llid = 0;
+               mesh_plink_frame_tx(sdata, PLINK_CLOSE, mgmt->sa, llid,
+                                   plid, reason);
+               return;
       } else if (!sta) {
               /* ftype == PLINK_OPEN */
               u32 rates;
@@ -522,7 +531,7 @@ void mesh_rx_plink_frame(struct
ieee80211_sub_if_data *sdata, struct ieee80211_m
               }
               event = OPN_ACPT;
               spin_lock_bh(&sta->lock);
-       } else {
+       } else if (matches_local) {
               spin_lock_bh(&sta->lock);
               switch (ftype) {
               case PLINK_OPEN:
@@ -564,6 +573,8 @@ void mesh_rx_plink_frame(struct
ieee80211_sub_if_data *sdata, struct ieee80211_m
                       rcu_read_unlock();
                       return;
               }
+       } else {
+               spin_lock_bh(&sta->lock);
       }

       mpl_dbg("Mesh plink (peer, state, llid, plid, event): %pM %s %d %d %d\n",
---

Cheers,

Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux