diff mbox series

[2/2] mpdu: tolerate technically illegal but harmless cloned IEs

Message ID 20241005021929.2075817-3-vivek@collabora.com (mailing list archive)
State New
Headers show
Series [1/2] unit: add a test for harmless IE clones | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
prestwoj/iwd-ci-gitlint success GitLint

Commit Message

Vivek Das Mohapatra Oct. 5, 2024, 2:19 a.m. UTC
---
 src/ie.h   | 17 +++++++++++++++++
 src/mpdu.c | 10 +++++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)

Comments

Denis Kenzior Oct. 7, 2024, 6:27 p.m. UTC | #1
Hi Vivek,

On 10/4/24 9:19 PM, Vivek Das Mohapatra wrote:
> ---
>   src/ie.h   | 17 +++++++++++++++++
>   src/mpdu.c | 10 +++++++++-
>   2 files changed, 26 insertions(+), 1 deletion(-)
> 

<snip>

> @@ -398,9 +398,17 @@ static bool validate_mgmt_ies(const uint8_t *ies, size_t ies_len,
>   
>   			memcpy(&clone, &iter, sizeof(clone));
>   
> +			/*
> +			 * Some APs send completely identical duplicate IEs:
> +			 * Since these are harmless (and ignored by us) we're
> +			 * going to allow them here for interoperability.
> +			 */
>   			while (ie_tlv_iter_next(&clone)) {
> -				if (ie_tlv_iter_get_tag(&clone) != tag)
> +				if (ie_tlv_iter_get_tag(&clone) != tag) {
>   					continue;
> +				} else if (ie_tlv_iter_data_eq(&iter, &clone)) {
> +					continue;
> +				}

No need for {}.  I took these out and applied.  Thanks.

Regards,
-Denis
Vivek Das Mohapatra Oct. 7, 2024, 6:29 p.m. UTC | #2
On 07/10/2024 19:27, Denis Kenzior wrote:
> Hi Vivek,
> 
> On 10/4/24 9:19 PM, Vivek Das Mohapatra wrote:
>> ---
>>   src/ie.h   | 17 +++++++++++++++++
>>   src/mpdu.c | 10 +++++++++-
>>   2 files changed, 26 insertions(+), 1 deletion(-)

[cut]

> 
> No need for {}.  I took these out and applied.  Thanks.

Thanks! I wasn't sure what the house style would be there.
diff mbox series

Patch

diff --git a/src/ie.h b/src/ie.h
index 4498785a..bc750696 100644
--- a/src/ie.h
+++ b/src/ie.h
@@ -598,6 +598,23 @@  static inline const unsigned char *ie_tlv_iter_get_data(
 	return iter->data;
 }
 
+static inline bool ie_tlv_iter_data_eq(struct ie_tlv_iter *a,
+					struct ie_tlv_iter *b)
+{
+	if (a == b)
+		return true;
+
+	if (a == NULL || b == NULL)
+		return false;
+
+	if (ie_tlv_iter_get_length(a) != ie_tlv_iter_get_length(b))
+		return false;
+
+	return memcmp(ie_tlv_iter_get_data(a),
+			ie_tlv_iter_get_data(b),
+			ie_tlv_iter_get_length(a)) == 0;
+}
+
 void *ie_tlv_extract_wsc_payload(const uint8_t *ies, size_t len,
 							ssize_t *out_len);
 void *ie_tlv_encapsulate_wsc_payload(const uint8_t *data, size_t len,
diff --git a/src/mpdu.c b/src/mpdu.c
index 9d0409d2..09df696e 100644
--- a/src/mpdu.c
+++ b/src/mpdu.c
@@ -398,9 +398,17 @@  static bool validate_mgmt_ies(const uint8_t *ies, size_t ies_len,
 
 			memcpy(&clone, &iter, sizeof(clone));
 
+			/*
+			 * Some APs send completely identical duplicate IEs:
+			 * Since these are harmless (and ignored by us) we're
+			 * going to allow them here for interoperability.
+			 */
 			while (ie_tlv_iter_next(&clone)) {
-				if (ie_tlv_iter_get_tag(&clone) != tag)
+				if (ie_tlv_iter_get_tag(&clone) != tag) {
 					continue;
+				} else if (ie_tlv_iter_data_eq(&iter, &clone)) {
+					continue;
+				}
 
 				return false;
 			}