[PATCH 1/4] monitor: fix buffer overrun parsing country IE

[PATCH 1/4] monitor: fix buffer overrun parsing country IE

[James Prestwood]

The country IE can sometimes have a zero pad byte at the end for
alignment. This was not being checked for which caused the loop
to go past the end of the IE and print an entry for channel 0
(the pad byte) plus some garbage data.

Fix this by checking for the pad byte explicitly which skips the
print and terminates the loop.
---
 monitor/nlmon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/monitor/nlmon.c b/monitor/nlmon.c
index 9694cfd1..652dea96 100644
--- a/monitor/nlmon.c
+++ b/monitor/nlmon.c
@@ -494,7 +494,7 @@ static void print_ie_country(unsigned int level, const char *label,
 			if (code[i + 2] < 32)
 				print_attr(level + 1, "%27c (air propagation "
 					"time %2d µs)", ' ', 3 * code[i + 2]);
-		} else {
+		} else if (code[i] != 0) {
 			print_attr(level + 1, "First channel %3d number of "
 				"channels %2d max tx power %2d dBm",
 				code[i], code[i + 1], code[i + 2]);
-- 
2.34.3

[PATCH 2/4] unit: fix test-band for 6ghz frequencies

[James Prestwood]

The 6GHz test was not incrementing the frequencies properly which
was resulting in invalid frequencies, but since the conversion
API was never linked to E-4 the test was still passing.
---
 unit/test-band.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/unit/test-band.c b/unit/test-band.c
index 90b48f9f..c47069f7 100644
--- a/unit/test-band.c
+++ b/unit/test-band.c
@@ -644,7 +644,7 @@ static void test_6ghz_freqs(const void *data)
 	uint32_t i;
 	enum band_freq band;
 
-	for (i = 5955; i < 7115; i += 5) {
+	for (i = 5955; i < 7115; i += 20) {
 		assert(band_freq_to_channel(i, &band) != 0);
 		assert(band == BAND_FREQ_6_GHZ);
 	}
-- 
2.34.3

[PATCH 3/4] band: validate channel/freq conversions with E-4

[James Prestwood]

IWD's channel/frequency conversions use simple math to convert and
have very minimal checks to ensure the input is valid. This can
lead to some channels/frequencies being calculated which are not
in IWD's E-4 table, specifically in the 5GHz band.

This is especially noticable using mac80211_hwsim which includes
some obscure high 5ghz frequencies which are not part of the 802.11
spec.

To fix this calculate the frequency or channel then iterate E-4
operating classes to check that the value actually matches a class.
---
 src/band.c | 79 +++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 54 insertions(+), 25 deletions(-)

diff --git a/src/band.c b/src/band.c
index c89762a9..27b4d174 100644
--- a/src/band.c
+++ b/src/band.c
@@ -1283,6 +1283,8 @@ int band_freq_to_ht_chandef(uint32_t freq, const struct band_freq_attrs *attr,
 
 uint8_t band_freq_to_channel(uint32_t freq, enum band_freq *out_band)
 {
+	unsigned int i;
+	enum band_freq band = 0;
 	uint32_t channel = 0;
 
 	if (freq >= 2412 && freq <= 2484) {
@@ -1297,10 +1299,8 @@ uint8_t band_freq_to_channel(uint32_t freq, enum band_freq *out_band)
 			channel /= 5;
 		}
 
-		if (out_band)
-			*out_band = BAND_FREQ_2_4_GHZ;
-
-		return channel;
+		band = BAND_FREQ_2_4_GHZ;
+		goto check_e4;
 	}
 
 	if (freq >= 5005 && freq < 5900) {
@@ -1309,10 +1309,9 @@ uint8_t band_freq_to_channel(uint32_t freq, enum band_freq *out_band)
 
 		channel = (freq - 5000) / 5;
 
-		if (out_band)
-			*out_band = BAND_FREQ_5_GHZ;
+		band = BAND_FREQ_5_GHZ;
 
-		return channel;
+		goto check_e4;
 	}
 
 	if (freq >= 4905 && freq < 5000) {
@@ -1321,10 +1320,9 @@ uint8_t band_freq_to_channel(uint32_t freq, enum band_freq *out_band)
 
 		channel = (freq - 4000) / 5;
 
-		if (out_band)
-			*out_band = BAND_FREQ_5_GHZ;
+		band = BAND_FREQ_5_GHZ;
 
-		return channel;
+		goto check_e4;
 	}
 
 	if (freq > 5950 && freq <= 7115) {
@@ -1333,17 +1331,31 @@ uint8_t band_freq_to_channel(uint32_t freq, enum band_freq *out_band)
 
 		channel = (freq - 5950) / 5;
 
-		if (out_band)
-			*out_band = BAND_FREQ_6_GHZ;
+		band = BAND_FREQ_6_GHZ;
 
-		return channel;
+		goto check_e4;
 	}
 
 	if (freq == 5935) {
-		if (out_band)
-			*out_band = BAND_FREQ_6_GHZ;
+		band = BAND_FREQ_6_GHZ;
+		channel = 2;
+	}
+
+	if (!band || !channel)
+		return 0;
+
+check_e4:
+	for (i = 0; i < L_ARRAY_SIZE(e4_operating_classes); i++) {
+		const struct operating_class_info *info =
+						&e4_operating_classes[i];
+
+		if (e4_has_frequency(info, freq) == 0 ||
+					e4_has_ccfi(info, freq) == 0) {
+			if (out_band)
+				*out_band = band;
 
-		return 2;
+			return channel;
+		}
 	}
 
 	return 0;
@@ -1351,26 +1363,33 @@ uint8_t band_freq_to_channel(uint32_t freq, enum band_freq *out_band)
 
 uint32_t band_channel_to_freq(uint8_t channel, enum band_freq band)
 {
+	unsigned int i;
+	uint32_t freq = 0;
+
 	if (band == BAND_FREQ_2_4_GHZ) {
 		if (channel >= 1 && channel <= 13)
-			return 2407 + 5 * channel;
+			freq = 2407 + 5 * channel;
+		else if (channel == 14)
+			freq = 2484;
 
-		if (channel == 14)
-			return 2484;
+		goto check_e4;
 	}
 
 	if (band == BAND_FREQ_5_GHZ) {
 		if (channel >= 1 && channel <= 179)
-			return 5000 + 5 * channel;
+			freq = 5000 + 5 * channel;
+		else if (channel >= 181 && channel <= 199)
+			freq = 4000 + 5 * channel;
 
-		if (channel >= 181 && channel <= 199)
-			return 4000 + 5 * channel;
+		goto check_e4;
 	}
 
 	if (band == BAND_FREQ_6_GHZ) {
 		/* operating class 136 */
-		if (channel == 2)
-			return 5935;
+		if (channel == 2) {
+			freq = 5935;
+			goto check_e4;
+		}
 
 		/* Channels increment by 4, starting with 1 */
 		if (channel % 4 != 1)
@@ -1380,7 +1399,17 @@ uint32_t band_channel_to_freq(uint8_t channel, enum band_freq band)
 			return 0;
 
 		/* operating classes 131, 132, 133, 134, 135 */
-		return 5950 + 5 * channel;
+		freq = 5950 + 5 * channel;
+	}
+
+check_e4:
+	for (i = 0; i < L_ARRAY_SIZE(e4_operating_classes); i++) {
+		const struct operating_class_info *info =
+						&e4_operating_classes[i];
+
+		if (e4_has_frequency(info, freq) == 0 ||
+					e4_has_ccfi(info, freq) == 0)
+			return freq;
 	}
 
 	return 0;
-- 
2.34.3

[PATCH 4/4] unit: add invalid channels/freqs to test-band

[James Prestwood]

Tests some channels and frequencies that are not in E-4 and
would pass the conversion without validation.
---
 unit/test-band.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/unit/test-band.c b/unit/test-band.c
index c47069f7..caddbcdd 100644
--- a/unit/test-band.c
+++ b/unit/test-band.c
@@ -650,6 +650,26 @@ static void test_6ghz_freqs(const void *data)
 	}
 }
 
+static void test_conversions(const void *data)
+{
+	/*
+	 * Test a few invalid channels/frequencies that appear valid but are
+	 * not in the E-4 table. The checks in band.c seem to cover 2.4Ghz and
+	 * 6Ghz very well since there are no gaps, but the 5GHz band has some
+	 * segmentation.
+	 */
+
+	/* Gap in 5GHz channels between 68 and 96 */
+	assert(!band_channel_to_freq(72, BAND_FREQ_5_GHZ));
+	assert(!band_freq_to_channel(5360, NULL));
+
+	/* Invalid channel using 4000mhz starting frequency */
+	assert(!band_channel_to_freq(183, BAND_FREQ_5_GHZ));
+	assert(!band_freq_to_channel(4915, NULL));
+
+	assert(!band_channel_to_freq(192, BAND_FREQ_5_GHZ));
+}
+
 int main(int argc, char *argv[])
 {
 	l_test_init(&argc, &argv);
@@ -715,5 +735,7 @@ int main(int argc, char *argv[])
 	l_test_add("/band/6ghz/channels", test_6ghz_channels, NULL);
 	l_test_add("/band/6ghz/freq", test_6ghz_freqs, NULL);
 
+	l_test_add("/band/conversions", test_conversions, NULL);
+
 	return l_test_run();
 }
-- 
2.34.3

Re: [PATCH 1/4] monitor: fix buffer overrun parsing country IE

[Denis Kenzior]

Hi James,

On 12/29/22 18:24, James Prestwood wrote:
> The country IE can sometimes have a zero pad byte at the end for
> alignment. This was not being checked for which caused the loop
> to go past the end of the IE and print an entry for channel 0
> (the pad byte) plus some garbage data.
> 
> Fix this by checking for the pad byte explicitly which skips the
> print and terminates the loop.
> ---
>   monitor/nlmon.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 

All applied, thanks.

Regards,
-Denis