Re: [PATCH 21/27] qtnfmac: extend "IE set" TLV to include frame type info
From: Sergey Matyukevich <hidden>
Date: 2017-08-30 12:07:32
- 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