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