[PATCH v3 2/3] wifi: nxpwifi: read the 11AX_CFG extension ID at the right offset

[Yehyeong Lee]

nxpwifi_ret_11ax_cfg() reads the extension ID of a TLV one byte past
where it is:

	ext_id = *((u8 *)header + sizeof(*header) + 1);

Everything else in the driver puts the extension ID at the first payload
byte, sizeof(*header) in. The send side of this very command does:

	memcpy(he_cfg->tlv + sizeof(*header),
	       &ax_cfg->he_cap_cfg.ext_id,
	       ax_cfg->he_cap_cfg.len);

struct nxpwifi_11ax_he_cap_cfg has ext_id at offset 4, after the u16 id
and the u16 len, and so do struct nxpwifi_ie_types_he_cap and struct
hw_spec_extension. nxpwifi_ret_sta_get_hw_spec() parses the same
TLV_TYPE_EXTENSION_ID out of a firmware response by casting to struct
hw_spec_extension and reading ->ext_id, i.e. at sizeof(*header). The
caller in nxpwifi_set_11ax_status() also writes the extension ID there,
at ax_cfg.data + 4. These two lines are the only place in the driver
that adds one.

The consequences are that the HE-capability test is made on the byte
after the extension ID, which is mac_cap_info[0], so a capability TLV is
rejected and some other TLV may be accepted in its place; that the copy
into he_cap->ext_id lands one byte off, leaving ext_id holding
mac_cap_info[0] and every later field shifted; and that the copy reads
one byte past the end of the TLV payload the response declared, since it
takes tlv_len bytes starting at payload[1]. The other copy in the same
function, into priv->user_he_cap, uses the unshifted offsets and so
disagrees with the first one. priv->user_he_cap is what
nxpwifi_update_11ax_cap() and nxpwifi_fill_he_cap_tlv() read back as a
struct nxpwifi_ie_types_he_cap, with ext_id at offset 4.

Drop the two plus-ones. With the correct offset the loop condition
"left_len > sizeof(*header)" already keeps the extension ID read inside
the declared response; it is the plus-one that needed one byte more than
the loop guarantees.

Also require a payload before reading the extension ID out of it. A
zero-length extension TLV carries no extension ID, and without the test
the first byte of whatever follows it is read as one.

Fixes: 73b01e57ed3e ("wifi: nxp: add nxpwifi driver for IW61x")
Signed-off-by: Yehyeong Lee <yhlee@xxxxxxxxxxxxxxxxxx>
---
 drivers/net/wireless/nxp/nxpwifi/11ax.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/nxp/nxpwifi/11ax.c b/drivers/net/wireless/nxp/nxpwifi/11ax.c
index d15495a85d2ba..a5278375329a4 100644
--- a/drivers/net/wireless/nxp/nxpwifi/11ax.c
+++ b/drivers/net/wireless/nxp/nxpwifi/11ax.c
@@ -219,13 +219,13 @@ int nxpwifi_ret_11ax_cfg(struct nxpwifi_private *priv,
 		tlv_type = le16_to_cpu(header->type);
 		tlv_len = le16_to_cpu(header->len);
 
-		if (tlv_type == TLV_TYPE_EXTENSION_ID) {
-			ext_id = *((u8 *)header + sizeof(*header) + 1);
+		if (tlv_type == TLV_TYPE_EXTENSION_ID && tlv_len >= 1) {
+			ext_id = *((u8 *)header + sizeof(*header));
 			if (ext_id == WLAN_EID_EXT_HE_CAPABILITY) {
 				he_cap->id = tlv_type;
 				he_cap->len = tlv_len;
 				memcpy((u8 *)&he_cap->ext_id,
-				       (u8 *)header + sizeof(*header) + 1,
+				       (u8 *)header + sizeof(*header),
 				       tlv_len);
 				if (he_cfg->band_config & BIT(1)) {
 					memcpy(priv->user_he_cap,
-- 
2.43.0