> - if (tlv_full_len > payload_len) {
> - pr_warn("VIF%u.%u: malformed TLV 0x%.2X; LEN: %u\n",
> - mac->macid, vif->vifid, tlv_type,
> - tlv_value_len);
> + if (tlv_full_len > payload_len)
> return -EINVAL;
> - }
Why drop this sanity check ?
> if (tlv_type == QTN_TLV_ID_IE_SET) {
> - sinfo.assoc_req_ies = tlv->val;
> - sinfo.assoc_req_ies_len = tlv_value_len;
> + const struct qlink_tlv_ie_set *ie_set;
> + unsigned int ie_len;
> +
> + if (payload_len < sizeof(*ie_set))
> + return -EINVAL;
> +
> + ie_set = (const struct qlink_tlv_ie_set *)tlv;
> + ie_len = tlv_value_len -
> + (sizeof(*ie_set) - sizeof(ie_set->hdr));
> +
> + if (ie_set->type == QLINK_IE_SET_ASSOC_REQ && ie_len) {
> + sinfo.assoc_req_ies = ie_set->ie_data;
> + sinfo.assoc_req_ies_len = ie_len;
> + }
> }
Does it make sense to keep QTN_TLV_ID_IE_SET here at all ?
Maybe replace it completely by qlink_tlv_ie_set with
QLINK_IE_SET_ASSOC_REQ type ? Also see the comment below
for the similar snippet in qtnf_event_handle_scan_results.
...
> - if (tlv_full_len > payload_len) {
> - pr_warn("VIF%u.%u: malformed TLV 0x%.2X; LEN: %u\n",
> - vif->mac->macid, vif->vifid, tlv_type,
> - tlv_value_len);
> + if (tlv_full_len > payload_len)
> return -EINVAL;
> - }
ditto
...
> if (tlv_type == QTN_TLV_ID_IE_SET) {
> - ies = tlv->val;
> - ies_len = tlv_value_len;
> + const struct qlink_tlv_ie_set *ie_set;
> + unsigned int ie_len;
> +
> + if (payload_len < sizeof(*ie_set))
> + return -EINVAL;
> +
> + ie_set = (const struct qlink_tlv_ie_set *)tlv;
> + ie_len = tlv_value_len -
> + (sizeof(*ie_set) - sizeof(ie_set->hdr));
> +
> + if (ie_len) {
> + ies = ie_set->ie_data;
> + ies_len = ie_len;
> + }
> }
> }
Two points here. First, it looks like there is a problem here inherited
from the existing implementation. We go through payload, but in fact we
pass to cfg80211_inform_bss only the last QTN_TLV_ID_IE_SET element.
Second, it looks like QTN_TLV_ID_IE_SET here should be treated in
the same way as in qtnf_event_handle_sta_assoc, to avoid confusion.
In other words, either we use only QTN_TLV_ID_IE_SET in both cases,
or switch to specific qlink_tlv_ie_set elements.
Thoughts ? Comments ?
Regards,
Sergey
