Hello Ilan Peer,
Commit de86c5f60839 ("wifi: mac80211: Add support for EPCS
configuration") from Feb 5, 2025 (linux-next), leads to the following
Smatch static checker warning:
net/mac80211/mlme.c:10707 ieee80211_ml_epcs()
warn: error code type promoted to positive: 'len'
net/mac80211/mlme.c
10663 static void ieee80211_ml_epcs(struct ieee80211_sub_if_data *sdata,
10664 struct ieee802_11_elems *elems)
10665 {
10666 const struct element *sub;
10667 size_t scratch_len = elems->ml_epcs_len;
10668 u8 *scratch __free(kfree) = kzalloc(scratch_len, GFP_KERNEL);
10669
10670 lockdep_assert_wiphy(sdata->local->hw.wiphy);
10671
10672 if (!ieee80211_vif_is_mld(&sdata->vif) || !elems->ml_epcs)
10673 return;
10674
10675 if (WARN_ON(!scratch))
10676 return;
10677
10678 /* Directly parse the sub elements as the common information doesn't
10679 * hold any useful information.
10680 */
10681 for_each_mle_subelement(sub, (const u8 *)elems->ml_epcs,
10682 elems->ml_epcs_len) {
10683 struct ieee80211_link_data *link;
10684 struct ieee802_11_elems *link_elems __free(kfree);
10685 u8 *pos = (void *)sub->data;
10686 u16 control;
10687 ssize_t len;
10688 u8 link_id;
10689
10690 if (sub->id != IEEE80211_MLE_SUBELEM_PER_STA_PROFILE)
10691 continue;
10692
10693 if (sub->datalen < sizeof(control))
10694 break;
10695
10696 control = get_unaligned_le16(pos);
10697 link_id = control & IEEE80211_MLE_STA_EPCS_CONTROL_LINK_ID;
10698
10699 link = sdata_dereference(sdata->link[link_id], sdata);
10700 if (!link)
10701 continue;
10702
10703 len = cfg80211_defragment_element(sub, (u8 *)elems->ml_epcs,
10704 elems->ml_epcs_len,
10705 scratch, scratch_len,
10706 IEEE80211_MLE_SUBELEM_FRAGMENT);
--> 10707 if (len < sizeof(control))
If cfg80211_defragment_element() returns a negative error code then
because of type promotion, the error codes are cast to size_t and become
high positive values. They are treated as success.
It would be easy enough to say:
if (len < (ssize_t)sizeof(control))
continue;
But I really am not sure the continues in this loop are correct.
Shouldn't we instead bail out and return at the first sign of corrupt
data?
10708 continue;
10709
10710 pos = scratch + sizeof(control);
10711 len -= sizeof(control);
10712
10713 link_elems = ieee802_11_parse_elems(pos, len, false, NULL);
10714 if (!link_elems)
10715 continue;
10716
10717 if (ieee80211_sta_wmm_params(sdata->local, link,
10718 link_elems->wmm_param,
10719 link_elems->wmm_param_len,
10720 link_elems->mu_edca_param_set))
10721 ieee80211_link_info_change_notify(sdata, link,
10722 BSS_CHANGED_QOS);
10723 }
10724 }
regards,
dan carpenter
