diff mbox series

[3/3] iw: scan: replace passed ie buffer with alias struct

Message ID 20240930181145.1043048-4-dylan.eskew@candelatech.com (mailing list archive)
State Changes Requested
Delegated to: Johannes Berg
Headers show
Series iw: scan: ie parsing restructure | expand

Commit Message

Dylan Eskew Sept. 30, 2024, 6:11 p.m. UTC 
Prevent needing to reparse the ie buffer by passing
the ieee80211_elems struct containing the alias ie
pointers.

Signed-off-by: Dylan Eskew <dylan.eskew@candelatech.com>
---
 scan.c | 106 ++++++++++++++++++++++++---------------------------------
 1 file changed, 44 insertions(+), 62 deletions(-)

Comments

Johannes Berg Nov. 7, 2024, 2:22 p.m. UTC | #1 
On Mon, 2024-09-30 at 11:11 -0700, Dylan Eskew wrote:
> Prevent needing to reparse the ie buffer by passing
> the ieee80211_elems struct containing the alias ie
> pointers.
> 

I think I see why you'd want the previous and this patch, and it's OK
for certain elements (such as the VHT capability or HE capability where
you'd need it), but in general it can't work this way.

We could either make the parsing even more generic to be able to deal
with elements occurring multiple times, or, perhaps more plausibly, have
a context with only what we need (VHT capability seen here, HE
capability for EHT, ...) filled by a pre-parsing step, and then pass
that context around.

In practice for HE/EHT we could also just pass a context around and say
it's an error if e.g. the EHT capabilities element is in the list
_before_ the HE capabilities element, since by spec that's not supposed
to happen. So we could just have the HE capabilities parsing fill the
context with the necessary information about HE (and set a validity
flag) and then check that it was already found when we get to EHT later.

However, that doesn't work for the case with capabilities/VHT ... where
the parsing of the capabilities element is actually referencing the VHT
element _later_ in the frame.

johannes
Dylan Eskew Nov. 7, 2024, 2:43 p.m. UTC | #2 
Thanks for the feedback. I didn't necessarily realize that elements
might appear multiple times in a frame, but that information paired
with your implementation advice gives me plenty to figure something
better.

-- Dylan

On 11/7/24 6:22 AM, Johannes Berg wrote:
> On Mon, 2024-09-30 at 11:11 -0700, Dylan Eskew wrote:
>> Prevent needing to reparse the ie buffer by passing
>> the ieee80211_elems struct containing the alias ie
>> pointers.
>>
> I think I see why you'd want the previous and this patch, and it's OK
> for certain elements (such as the VHT capability or HE capability where
> you'd need it), but in general it can't work this way.
>
> We could either make the parsing even more generic to be able to deal
> with elements occurring multiple times, or, perhaps more plausibly, have
> a context with only what we need (VHT capability seen here, HE
> capability for EHT, ...) filled by a pre-parsing step, and then pass
> that context around.
>
> In practice for HE/EHT we could also just pass a context around and say
> it's an error if e.g. the EHT capabilities element is in the list
> _before_ the HE capabilities element, since by spec that's not supposed
> to happen. So we could just have the HE capabilities parsing fill the
> context with the necessary information about HE (and set a validity
> flag) and then check that it was already found when we get to EHT later.
>
> However, that doesn't work for the case with capabilities/VHT ... where
> the parsing of the capabilities element is actually referencing the VHT
> element _later_ in the frame.
>
> johannes
diff mbox series

Patch

diff --git a/scan.c b/scan.c
index 6fdaf0b..3210ccc 100644
--- a/scan.c
+++ b/scan.c
@@ -554,13 +554,8 @@  static void tab_on_first(bool *first)
 		*first = false;
 }
 
-struct print_ies_data {
-	unsigned char *ie;
-	int ielen;
-};
-
 static void print_ssid(const uint8_t type, uint8_t len, const uint8_t *data,
-		       const struct print_ies_data *ie_buffer)
+		       const struct ieee80211_elems *elems)
 {
 	printf(" ");
 	print_ssid_escaped(len, data);
@@ -572,7 +567,7 @@  static void print_ssid(const uint8_t type, uint8_t len, const uint8_t *data,
 
 static void print_supprates(const uint8_t type, uint8_t len,
 			    const uint8_t *data,
-			    const struct print_ies_data *ie_buffer)
+			    const struct ieee80211_elems *elems)
 {
 	int i;
 
@@ -595,7 +590,7 @@  static void print_supprates(const uint8_t type, uint8_t len,
 
 static void print_rm_enabled_capabilities(const uint8_t type, uint8_t len,
 			    const uint8_t *data,
-			    const struct print_ies_data *ie_buffer)
+			    const struct ieee80211_elems *elems)
 {
 	__u64 capa = ((__u64) data[0]) |
 		     ((__u64) data[1]) << 8 |
@@ -649,7 +644,7 @@  static void print_rm_enabled_capabilities(const uint8_t type, uint8_t len,
 }
 
 static void print_ds(const uint8_t type, uint8_t len, const uint8_t *data,
-		     const struct print_ies_data *ie_buffer)
+		     const struct ieee80211_elems *elems)
 {
 	printf(" channel %d\n", data[0]);
 }
@@ -669,7 +664,7 @@  static const char *country_env_str(char environment)
 }
 
 static void print_country(const uint8_t type, uint8_t len, const uint8_t *data,
-			  const struct print_ies_data *ie_buffer)
+			  const struct ieee80211_elems *elems)
 {
 	printf(" %.*s", 2, data);
 
@@ -716,21 +711,21 @@  static void print_country(const uint8_t type, uint8_t len, const uint8_t *data,
 
 static void print_powerconstraint(const uint8_t type, uint8_t len,
 				  const uint8_t *data,
-				  const struct print_ies_data *ie_buffer)
+				  const struct ieee80211_elems *elems)
 {
 	printf(" %d dB\n", data[0]);
 }
 
 static void print_tpcreport(const uint8_t type, uint8_t len,
 			    const uint8_t *data,
-			    const struct print_ies_data *ie_buffer)
+			    const struct ieee80211_elems *elems)
 {
 	printf(" TX power: %d dBm\n", data[0]);
 	/* printf(" Link Margin (%d dB) is reserved in Beacons\n", data[1]); */
 }
 
 static void print_erp(const uint8_t type, uint8_t len, const uint8_t *data,
-		      const struct print_ies_data *ie_buffer)
+		      const struct ieee80211_elems *elems)
 {
 	if (data[0] == 0x00)
 		printf(" <no flags>");
@@ -744,7 +739,7 @@  static void print_erp(const uint8_t type, uint8_t len, const uint8_t *data,
 }
 
 static void print_ap_channel_report(const uint8_t type, uint8_t len, const uint8_t *data,
-				    const struct print_ies_data *ie_buffer)
+				    const struct ieee80211_elems *elems)
 {
 	uint8_t oper_class = data[0];
 	int i;
@@ -1084,13 +1079,13 @@  static void print_osen_ie(const char *defcipher, const char *defauth,
 }
 
 static void print_rsn(const uint8_t type, uint8_t len, const uint8_t *data,
-		      const struct print_ies_data *ie_buffer)
+		      const struct ieee80211_elems *elems)
 {
 	print_rsn_ie("CCMP", "IEEE 802.1X", len, data);
 }
 
 static void print_ht_capa(const uint8_t type, uint8_t len, const uint8_t *data,
-			  const struct print_ies_data *ie_buffer)
+			  const struct ieee80211_elems *elems)
 {
 	printf("\n");
 	print_ht_capability(data[0] | (data[1] << 8));
@@ -1135,7 +1130,7 @@  static const char* vgroup_11u(uint8_t t)
 
 static void print_interworking(const uint8_t type, uint8_t len,
 			       const uint8_t *data,
-			       const struct print_ies_data *ie_buffer)
+			       const struct ieee80211_elems *elems)
 {
 	/* See Section 7.3.2.92 in the 802.11u spec. */
 	printf("\n");
@@ -1168,7 +1163,7 @@  static void print_interworking(const uint8_t type, uint8_t len,
 
 static void print_11u_advert(const uint8_t type, uint8_t len,
 			     const uint8_t *data,
-			     const struct print_ies_data *ie_buffer)
+			     const struct ieee80211_elems *elems)
 {
 	/* See Section 7.3.2.93 in the 802.11u spec. */
 	/* TODO: This code below does not decode private protocol IDs */
@@ -1201,7 +1196,7 @@  static void print_11u_advert(const uint8_t type, uint8_t len,
 }
 
 static void print_11u_rcon(const uint8_t type, uint8_t len, const uint8_t *data,
-			   const struct print_ies_data *ie_buffer)
+			   const struct ieee80211_elems *elems)
 {
 	/* See Section 7.3.2.96 in the 802.11u spec. */
 	int idx = 0;
@@ -1254,7 +1249,7 @@  static void print_11u_rcon(const uint8_t type, uint8_t len, const uint8_t *data,
 
 static void print_tx_power_envelope(const uint8_t type, uint8_t len,
 				    const uint8_t *data,
-				    const struct print_ies_data *ie_buffer)
+				    const struct ieee80211_elems *elems)
 {
 	const uint8_t local_max_tx_power_count = data[0] & 7;
 	const uint8_t local_max_tx_power_unit_interp = (data[0] >> 3) & 7;
@@ -1290,7 +1285,7 @@  static const char *ht_secondary_offset[4] = {
 };
 
 static void print_ht_op(const uint8_t type, uint8_t len, const uint8_t *data,
-			const struct print_ies_data *ie_buffer)
+			const struct ieee80211_elems *elems)
 {
 	static const char *protection[4] = {
 		"no",
@@ -1322,21 +1317,11 @@  static void print_ht_op(const uint8_t type, uint8_t len, const uint8_t *data,
 
 static void print_capabilities(const uint8_t type, uint8_t len,
 			       const uint8_t *data,
-			       const struct print_ies_data *ie_buffer)
+			       const struct ieee80211_elems *elems)
 {
 	int i, base, bit, si_duration = 0, max_amsdu = 0;
-	bool s_psmp_support = false, is_vht_cap = false;
-	unsigned char *ie = ie_buffer->ie;
-	int ielen = ie_buffer->ielen;
-
-	while (ielen >= 2 && ielen >= ie[1]) {
-		if (ie[0] == 191) {
-			is_vht_cap = true;
-			break;
-		}
-		ielen -= ie[1] + 2;
-		ie += ie[1] + 2;
-	}
+	bool is_vht_cap = (elems->lengths[EID_VHT_CAPABILITY] > 0);
+	bool s_psmp_support = false;
 
 	for (i = 0; i < len; i++) {
 		base = i * 8;
@@ -1486,7 +1471,7 @@  static void print_capabilities(const uint8_t type, uint8_t len,
 }
 
 static void print_tim(const uint8_t type, uint8_t len, const uint8_t *data,
-		      const struct print_ies_data *ie_buffer)
+		      const struct ieee80211_elems *elems)
 {
 	printf(" DTIM Count %u DTIM Period %u Bitmap Control 0x%x "
 	       "Bitmap[0] 0x%x",
@@ -1497,13 +1482,13 @@  static void print_tim(const uint8_t type, uint8_t len, const uint8_t *data,
 }
 
 static void print_ibssatim(const uint8_t type, uint8_t len, const uint8_t *data,
-			   const struct print_ies_data *ie_buffer)
+			   const struct ieee80211_elems *elems)
 {
 	printf(" %d TUs\n", (data[1] << 8) + data[0]);
 }
 
 static void print_vht_capa(const uint8_t type, uint8_t len, const uint8_t *data,
-			   const struct print_ies_data *ie_buffer)
+			   const struct ieee80211_elems *elems)
 {
 	printf("\n");
 	print_vht_info((__u32) data[0] | ((__u32)data[1] << 8) |
@@ -1512,7 +1497,7 @@  static void print_vht_capa(const uint8_t type, uint8_t len, const uint8_t *data,
 }
 
 static void print_vht_oper(const uint8_t type, uint8_t len, const uint8_t *data,
-			   const struct print_ies_data *ie_buffer)
+			   const struct ieee80211_elems *elems)
 {
 	const char *chandwidths[] = {
 		[0] = "20 or 40 MHz",
@@ -1531,7 +1516,7 @@  static void print_vht_oper(const uint8_t type, uint8_t len, const uint8_t *data,
 
 static void print_supp_op_classes(const uint8_t type, uint8_t len,
 				  const uint8_t *data,
-				  const struct print_ies_data *ie_buffer)
+				  const struct ieee80211_elems *elems)
 {
 	uint8_t *p = (uint8_t*) data;
 	const uint8_t *next_data = p + len;
@@ -1565,7 +1550,7 @@  static void print_supp_op_classes(const uint8_t type, uint8_t len,
 
 static void print_measurement_pilot_tx(const uint8_t type, uint8_t len,
 				       const uint8_t *data,
-				       const struct print_ies_data *ie_buffer)
+				       const struct ieee80211_elems *elems)
 {
 	uint8_t *p, len_remaining;
 
@@ -1614,7 +1599,7 @@  static void print_measurement_pilot_tx(const uint8_t type, uint8_t len,
 
 static void print_obss_scan_params(const uint8_t type, uint8_t len,
 				   const uint8_t *data,
-				   const struct print_ies_data *ie_buffer)
+				   const struct ieee80211_elems *elems)
 {
 	printf("\n");
 	printf("\t\t * passive dwell: %d TUs\n", (data[1] << 8) | data[0]);
@@ -1629,7 +1614,7 @@  static void print_obss_scan_params(const uint8_t type, uint8_t len,
 
 static void print_secchan_offs(const uint8_t type, uint8_t len,
 			       const uint8_t *data,
-			       const struct print_ies_data *ie_buffer)
+			       const struct ieee80211_elems *elems)
 {
 	if (data[0] < ARRAY_SIZE(ht_secondary_offset))
 		printf(" %s (%d)\n", ht_secondary_offset[data[0]], data[0]);
@@ -1638,7 +1623,7 @@  static void print_secchan_offs(const uint8_t type, uint8_t len,
 }
 
 static void print_bss_load(const uint8_t type, uint8_t len, const uint8_t *data,
-			   const struct print_ies_data *ie_buffer)
+			   const struct ieee80211_elems *elems)
 {
 	printf("\n");
 	printf("\t\t * station count: %d\n", (data[1] << 8) | data[0]);
@@ -1648,7 +1633,7 @@  static void print_bss_load(const uint8_t type, uint8_t len, const uint8_t *data,
 
 static void print_mesh_conf(const uint8_t type, uint8_t len,
 			    const uint8_t *data,
-			    const struct print_ies_data *ie_buffer)
+			    const struct ieee80211_elems *elems)
 {
 	printf("\n");
 	printf("\t\t * Active Path Selection Protocol ID: %d\n", data[0]);
@@ -1681,7 +1666,7 @@  static void print_mesh_conf(const uint8_t type, uint8_t len,
 
 static void print_s1g_capa(const uint8_t type, uint8_t len,
 			    const uint8_t *data,
-			    const struct print_ies_data *ie_buffer)
+			    const struct ieee80211_elems *elems)
 {
 	printf("\n");
 	print_s1g_capability(data);
@@ -1689,14 +1674,14 @@  static void print_s1g_capa(const uint8_t type, uint8_t len,
 
 static void print_short_beacon_int(const uint8_t type, uint8_t len,
 			    const uint8_t *data,
-			    const struct print_ies_data *ie_buffer)
+			    const struct ieee80211_elems *elems)
 {
 	printf(" %d\n", (data[1] << 8) | data[0]);
 }
 
 static void print_s1g_oper(const uint8_t type, uint8_t len,
 			    const uint8_t *data,
-			    const struct print_ies_data *ie_buffer)
+			    const struct ieee80211_elems *elems)
 {
 	int oper_ch_width, prim_ch_width;
 	int prim_ch_width_subfield = data[0] & 0x1;
@@ -1777,7 +1762,7 @@  static void print_s1g_oper(const uint8_t type, uint8_t len,
 struct ie_print {
 	const char *name;
 	void (*print)(const uint8_t type, uint8_t len, const uint8_t *data,
-		      const struct print_ies_data *ie_buffer);
+		      const struct ieee80211_elems *elems);
 	uint8_t minlen, maxlen;
 	uint8_t flags;
 };
@@ -1790,7 +1775,7 @@  struct element {
 
 static void print_ie(const struct ie_print *p, const uint8_t type, uint8_t len,
 		     const uint8_t *data,
-		     const struct print_ies_data *ie_buffer)
+		     const struct ieee80211_elems *elems)
 {
 	int i;
 
@@ -1811,7 +1796,7 @@  static void print_ie(const struct ie_print *p, const uint8_t type, uint8_t len,
 		return;
 	}
 
-	p->print(type, len, data, ie_buffer);
+	p->print(type, len, data, elems);
 }
 
 #define PRINT_IGN {		\
@@ -1873,14 +1858,14 @@  static const struct ie_print ieprinters[] = {
 };
 
 static void print_wifi_wpa(const uint8_t type, uint8_t len, const uint8_t *data,
-			   const struct print_ies_data *ie_buffer)
+			   const struct ieee80211_elems *elems)
 {
 	print_rsn_ie("TKIP", "IEEE 802.1X", len, data);
 }
 
 static void print_wifi_osen(const uint8_t type, uint8_t len,
 			    const uint8_t *data,
-			    const struct print_ies_data *ie_buffer)
+			    const struct ieee80211_elems *elems)
 {
 	print_osen_ie("OSEN", "OSEN", len, data);
 }
@@ -1928,7 +1913,7 @@  static bool print_wifi_wmm_param(const uint8_t *data, uint8_t len)
 }
 
 static void print_wifi_wmm(const uint8_t type, uint8_t len, const uint8_t *data,
-			   const struct print_ies_data *ie_buffer)
+			   const struct ieee80211_elems *elems)
 {
 	int i;
 
@@ -1971,7 +1956,7 @@  static const char * wifi_wps_dev_passwd_id(uint16_t id)
 }
 
 static void print_wifi_wps(const uint8_t type, uint8_t len, const uint8_t *data,
-			   const struct print_ies_data *ie_buffer)
+			   const struct ieee80211_elems *elems)
 {
 	bool first = true;
 	__u16 subtype, sublen;
@@ -2211,7 +2196,7 @@  static const struct ie_print wifiprinters[] = {
 
 static inline void print_p2p(const uint8_t type, uint8_t len,
 			     const uint8_t *data,
-			     const struct print_ies_data *ie_buffer)
+			     const struct ieee80211_elems *elems)
 {
 	bool first = true;
 	__u8 subtype;
@@ -2293,7 +2278,7 @@  static inline void print_p2p(const uint8_t type, uint8_t len,
 
 static inline void print_hs20_ind(const uint8_t type, uint8_t len,
 				  const uint8_t *data,
-				  const struct print_ies_data *ie_buffer)
+				  const struct ieee80211_elems *elems)
 {
 	/* I can't find the spec for this...just going off what wireshark uses. */
 	printf("\n");
@@ -2305,7 +2290,7 @@  static inline void print_hs20_ind(const uint8_t type, uint8_t len,
 
 static void print_wifi_owe_tarns(const uint8_t type, uint8_t len,
 				 const uint8_t *data,
-				 const struct print_ies_data *ie_buffer)
+				 const struct ieee80211_elems *elems)
 {
 	char mac_addr[20];
 	int ssid_len;
@@ -2398,7 +2383,7 @@  static void print_vendor(unsigned char len, const unsigned char *data,
 }
 
 static void print_he_capa(const uint8_t type, uint8_t len, const uint8_t *data,
-			  const struct print_ies_data *ie_buffer)
+			  const struct ieee80211_elems *elems)
 {
 	printf("\n");
 	print_he_capability(data, len);
@@ -2495,9 +2480,6 @@  static int parse_ies(unsigned char *ie, int ielen, bool unknown,
 void print_ies(unsigned char *ie, int ielen, bool unknown,
 	       enum print_ie_type ptype)
 {
-	struct print_ies_data ie_buffer = {
-		.ie = ie,
-		.ielen = ielen };
 	struct ieee80211_elems elems = { };
 	unsigned i;
 
@@ -2513,7 +2495,7 @@  void print_ies(unsigned char *ie, int ielen, bool unknown,
 		    ieprinters[i].flags & BIT(ptype) &&
 		    elems.lengths[i] > 0) {
 			print_ie(&ieprinters[i], i, elems.lengths[i],
-				 elems.ies[i], &ie_buffer);
+				 elems.ies[i], &elems);
 		}
 		else if (i == EID_VENDOR) {
 			print_vendor(elems.lengths[i], elems.ies[i],