mac80211: fix rx data handling for non-data frames on multiple vifs

PATCH In review Felix Fietkau 2 participants 2 messages Jan 21, 2010 – Jan 22, 2010 mac80211

Share & tools

Felix Fietkau

author

2010-01-21 23:36 UTC #0 lore raw

PATCH 1/1 mac80211: fix rx data handling for non-data frames on multiple vifs

The loop that passes non-data frames to all relevant vifs inside the __ieee80211_rx_handle_packet keeps a pointer to the previous sdata to avoid having to make unnecessary copies of the frame it's handling. This led to a bug that caused it to apply the ieee80211_rx_data state to the wrong interface, thereby either missing the rx.sta pointer or having it assigned where it shouldn't be.
This breaks (among other things) aggregation on some vifs, as action frame exchages are dropped to the cooked monitor interface due to rx->sta being NULL.
Fix this by restructuring the loop so that it prepares the rx data just before making the skb copy and calling the rx handlers.

Cc [email protected]

Signed-off-by Felix Fietkau <[email protected]>

---

net/mac80211/rx.c +28 ~~-17~~

@@ -2348,22 +2348,6 @@ static void __ieee80211_rx_handle_packet
 			    sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
 				continue;
 
-			rx.sta = sta_info_get(sdata, hdr->addr2);
-
-			rx.flags |= IEEE80211_RX_RA_MATCH;
-			prepares = prepare_for_handlers(sdata, &rx, hdr);
-
-			if (!prepares)
-				continue;
-
-			if (status->flag & RX_FLAG_MMIC_ERROR) {
-				rx.sdata = sdata;
-				if (rx.flags & IEEE80211_RX_RA_MATCH)
-					ieee80211_rx_michael_mic_report(hdr,
-									&rx);
-				continue;
-			}
-
 			/*
 			 * frame is destined for this interface, but if it's
 			 * not also for the previous one we handle that after

@@ -2375,6 +2359,22 @@ static void __ieee80211_rx_handle_packet
 				continue;
 			}
 
+			rx.sta = sta_info_get(prev, hdr->addr2);
+
+			rx.flags |= IEEE80211_RX_RA_MATCH;
+			prepares = prepare_for_handlers(prev, &rx, hdr);
+
+			if (!prepares)
+				goto next;
+
+			if (status->flag & RX_FLAG_MMIC_ERROR) {
+				rx.sdata = prev;
+				if (rx.flags & IEEE80211_RX_RA_MATCH)
+					ieee80211_rx_michael_mic_report(hdr,
+									&rx);
+				goto next;
+			}
+
 			/*
 			 * frame was destined for the previous interface
 			 * so invoke RX handlers for it

@@ -2387,11 +2387,22 @@ static void __ieee80211_rx_handle_packet
 					       "multicast frame for %s\n",
 					       wiphy_name(local->hw.wiphy),
 					       prev->name);
-				continue;
+				goto next;
 			}
 			ieee80211_invoke_rx_handlers(prev, &rx, skb_new, rate);
+next:
 			prev = sdata;
 		}
+
+		if (prev) {
+			rx.sta = sta_info_get(prev, hdr->addr2);
+
+			rx.flags |= IEEE80211_RX_RA_MATCH;
+			prepares = prepare_for_handlers(prev, &rx, hdr);
+
+			if (!prepares)
+				prev = NULL;
+		}
 	}
 	if (prev)
 		ieee80211_invoke_rx_handlers(prev, &rx, skb, rate);

Johannes Berg

maintainer

2010-01-22 09:27 UTC #1 lore raw

Quoted 68 lines · Felix Fietkau

Fix this by restructuring the loop so that it prepares the rx data just before making the skb copy and calling the rx handlers.

Cc [email protected]

Signed-off-by Felix Fietkau <[email protected]>

---

net/mac80211/rx.c +21 ~~-17~~

@@ -2348,22 +2348,6 @@ static void __ieee80211_rx_handle_packet
 			    sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
 				continue;
 
-			rx.sta = sta_info_get(sdata, hdr->addr2);
-
-			rx.flags |= IEEE80211_RX_RA_MATCH;
-			prepares = prepare_for_handlers(sdata, &rx, hdr);
-
-			if (!prepares)
-				continue;
-
-			if (status->flag & RX_FLAG_MMIC_ERROR) {
-				rx.sdata = sdata;
-				if (rx.flags & IEEE80211_RX_RA_MATCH)
-					ieee80211_rx_michael_mic_report(hdr,
-									&rx);
-				continue;
-			}
-
 			/*
 			 * frame is destined for this interface, but if it's
 			 * not also for the previous one we handle that after

@@ -2375,6 +2359,22 @@ static void __ieee80211_rx_handle_packet
 				continue;
 			}
 
+			rx.sta = sta_info_get(prev, hdr->addr2);
+
+			rx.flags |= IEEE80211_RX_RA_MATCH;
+			prepares = prepare_for_handlers(prev, &rx, hdr);
+
+			if (!prepares)
+				goto next;
+
+			if (status->flag & RX_FLAG_MMIC_ERROR) {
+				rx.sdata = prev;
+				if (rx.flags & IEEE80211_RX_RA_MATCH)
+					ieee80211_rx_michael_mic_report(hdr,
+									&rx);
+				goto next;
+			}
+
 			/*
 			 * frame was destined for the previous interface
 			 * so invoke RX handlers for it

@@ -2387,11 +2387,22 @@ static void __ieee80211_rx_handle_packet
 					       "multicast frame for %s\n",
 					       wiphy_name(local->hw.wiphy),
 					       prev->name);
-				continue;
+				goto next;
 			}
 			ieee80211_invoke_rx_handlers(prev, &rx, skb_new, rate);
+next:
 			prev = sdata;
 		}
+
+		if (prev) {
+			rx.sta = sta_info_get(prev, hdr->addr2);

net/mac80211/rx.c static void __ieee80211_rx_handle_packet · line 2408

Quoted 10 lines

net/mac80211/rx.c +7 -0

+
+			rx.flags |= IEEE80211_RX_RA_MATCH;
+			prepares = prepare_for_handlers(prev, &rx, hdr);
+
+			if (!prepares)
+				prev = NULL;
+		}
 	}
 	if (prev)
 		ieee80211_invoke_rx_handlers(prev, &rx, skb, rate);

That's kinda subtle, but looks correct at first and second glance, thanks.

johannes