On Fri, Nov 22, 2019 at 05:43:49PM +0800, qize wang wrote:
> case WLAN_EID_HT_CAPABILITY:
> - memcpy((u8 *)&sta_ptr->tdls_cap.ht_capb, pos,
^^^^
> + if (pos > end - sizeof(struct ieee80211_ht_cap) - 2)
> + return;
> + if (pos[1] != sizeof(struct ieee80211_ht_cap))
> + return;
> + /* copy the ie's value into ht_capb*/
> + memcpy((u8 *)&sta_ptr->tdls_cap.ht_capb, pos + 2,
^^^^^^^
I don't understand why we changed "pos" to "pos + 2". Presumably there
is a reason, but it needs to explained in the commit message.
> sizeof(struct ieee80211_ht_cap));
> sta_ptr->is_11n_enabled = 1;
> break;
> case WLAN_EID_HT_OPERATION:
> - memcpy(&sta_ptr->tdls_cap.ht_oper, pos,
^^^
> + if (pos > end -
> + sizeof(struct ieee80211_ht_operation) - 2)
> + return;
> + if (pos[1] != sizeof(struct ieee80211_ht_operation))
> + return;
> + /* copy the ie's value into ht_oper*/
> + memcpy(&sta_ptr->tdls_cap.ht_oper, pos + 2,
^^^^^^^
> sizeof(struct ieee80211_ht_operation));
> break;
> case WLAN_EID_BSS_COEX_2040:
> + if (pos > end - 3)
> + return;
> + if (pos[1] != 1)
> + return;
> sta_ptr->tdls_cap.coex_2040 = pos[2];
> break;
> case WLAN_EID_EXT_CAPABILITY:
> + if (pos > end - sizeof(struct ieee_types_header))
> + return;
> + if (pos[1] < sizeof(struct ieee_types_header))
> + return;
> + if (pos[1] > 8)
> + return;
> memcpy((u8 *)&sta_ptr->tdls_cap.extcap, pos,
> sizeof(struct ieee_types_header) +
> min_t(u8, pos[1], 8));
> break;
> case WLAN_EID_RSN:
> + if (pos > end - sizeof(struct ieee_types_header))
> + return;
> + if (pos[1] < sizeof(struct ieee_types_header))
> + return;
> + if (pos[1] > IEEE_MAX_IE_SIZE -
> + sizeof(struct ieee_types_header))
> + return;
> memcpy((u8 *)&sta_ptr->tdls_cap.rsn_ie, pos,
> sizeof(struct ieee_types_header) +
> min_t(u8, pos[1], IEEE_MAX_IE_SIZE -
> sizeof(struct ieee_types_header)));
> break;
> case WLAN_EID_QOS_CAPA:
> + if (pos > end - 3)
> + return;
> + if (pos[1] != 1)
> + return;
> sta_ptr->tdls_cap.qos_info = pos[2];
> break;
> case WLAN_EID_VHT_OPERATION:
> - if (priv->adapter->is_hw_11ac_capable)
> - memcpy(&sta_ptr->tdls_cap.vhtoper, pos,
^^^
> + if (priv->adapter->is_hw_11ac_capable) {
> + if (pos > end -
> + sizeof(struct ieee80211_vht_operation) - 2)
> + return;
> + if (pos[1] !=
> + sizeof(struct ieee80211_vht_operation))
> + return;
> + /* copy the ie's value into vhtoper*/
> + memcpy(&sta_ptr->tdls_cap.vhtoper, pos + 2,
^^^^^^^
> sizeof(struct ieee80211_vht_operation));
> + }
> break;
> case WLAN_EID_VHT_CAPABILITY:
> if (priv->adapter->is_hw_11ac_capable) {
> - memcpy((u8 *)&sta_ptr->tdls_cap.vhtcap, pos,
^^^
> + if (pos > end -
> + sizeof(struct ieee80211_vht_cap) - 2)
> + return;
> + if (pos[1] != sizeof(struct ieee80211_vht_cap))
> + return;
> + /* copy the ie's value into vhtcap*/
> + memcpy((u8 *)&sta_ptr->tdls_cap.vhtcap, pos + 2,
^^^^^^^
> sizeof(struct ieee80211_vht_cap));
> sta_ptr->is_11ac_enabled = 1;
> }
> break;
I was confused by the "- 2" as well in the earlier versions of this
patch but Marvell approved it so I assumed it was correct. It would be
nice if this patch were tested.
regards,
dan carpenter
