| Message ID | CAAMvbhGyheFdWSrDzM_i10n9s06n3G2wX6O_S68VUZyP-a9p+A@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | wifi: mac80211: add input validation to sta_stats_decode_rate() | expand |
On Mon, 2024-05-27 at 00:43 +0100, James Dutton wrote: > Validation is required as a result of parameters derived from > received wifi packets. I don't think I fully agree with that. First of all, this data is never actually directly derived from the wifi packet (certainly not any pointers or the band enum!), even the PLCP contains different encodings. Thus there's always already a translation in driver or firmware. Now of course we shouldn't trust firmware either, but even then there are a lot of places, I'd think this is better done at the driver level. johannes
On Mon, 27 May 2024 at 06:41, Johannes Berg <johannes@sipsolutions.net> wrote: > > On Mon, 2024-05-27 at 00:43 +0100, James Dutton wrote: > > Validation is required as a result of parameters derived from > > received wifi packets. > > I don't think I fully agree with that. First of all, this data is never > actually directly derived from the wifi packet (certainly not any > pointers or the band enum!), even the PLCP contains different encodings. > Thus there's always already a translation in driver or firmware. > > Now of course we shouldn't trust firmware either, but even then there > are a lot of places, I'd think this is better done at the driver level. > Hi johannes, You mention "certainly not any pointers or the band enum!". How certain are you about that statement? I ask because received wifi packets can and do result in unwelcome values for the pointers here. I did not say "directly derived". Kind Regards James
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c index da5fdd6f5c85..bab05c6b1bcc 100644 --- a/net/mac80211/sta_info.c +++ b/net/mac80211/sta_info.c @@ -2437,11 +2437,26 @@ static void sta_stats_decode_rate(struct ieee80211_local *local, u32 rate, int band = STA_STATS_GET(LEGACY_BAND, rate); int rate_idx = STA_STATS_GET(LEGACY_IDX, rate); + if (WARN_ON_ONCE(!local)) + break; + + if (WARN_ON_ONCE(!rinfo)) + break; + + if (WARN_ON_ONCE(band >= NUM_NL80211_BANDS)) + break; + sband = local->hw.wiphy->bands[band]; + if (WARN_ON_ONCE(!sband)) + break; + if (WARN_ON_ONCE(!sband->bitrates)) break; + if (WARN_ON_ONCE(rate_idx >= sband->n_bitrates)) + break; + brate = sband->bitrates[rate_idx].bitrate; if (rinfo->bw == RATE_INFO_BW_5)
Validation is required as a result of parameters derived from received wifi packets. Signed-off-by: James Courtier-Dutton <james.dutton@gmail.com> --- net/mac80211/sta_info.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) shift = 2;