| From 4b5dde2d6234ff5bc68e97e6901d1f2a0a7f3749 Mon Sep 17 00:00:00 2001 |
| From: Brian Norris <briannorris@chromium.org> |
| Date: Thu, 29 Jun 2017 18:23:54 -0700 |
| Subject: mwifiex: correct channel stat buffer overflows |
| |
| From: Brian Norris <briannorris@chromium.org> |
| |
| commit 4b5dde2d6234ff5bc68e97e6901d1f2a0a7f3749 upstream. |
| |
| mwifiex records information about various channels as it receives scan |
| information. It does this by appending to a buffer that was sized |
| to the max number of supported channels on any band, but there are |
| numerous problems: |
| |
| (a) scans can return info from more than one band (e.g., both 2.4 and 5 |
| GHz), so the determined "max" is not large enough |
| (b) some firmware appears to return multiple results for a given |
| channel, so the max *really* isn't large enough |
| (c) there is no bounds checking when stashing these stats, so problems |
| (a) and (b) can easily lead to buffer overflows |
| |
| Let's patch this by setting a slightly-more-correct max (that accounts |
| for a combination of both 2.4G and 5G bands) and adding a bounds check |
| when writing to our statistics buffer. |
| |
| Due to problem (b), we still might not properly report all known survey |
| information (e.g., with "iw <dev> survey dump"), since duplicate results |
| (or otherwise "larger than expected" results) will cause some |
| truncation. But that's a problem for a future bugfix. |
| |
| (And because of this known deficiency, only log the excess at the WARN |
| level, since that isn't visible by default in this driver and would |
| otherwise be a bit too noisy.) |
| |
| Fixes: bf35443314ac ("mwifiex: channel statistics support for mwifiex") |
| Cc: Avinash Patil <patila@marvell.com> |
| Cc: Xinming Hu <huxm@marvell.com> |
| Signed-off-by: Brian Norris <briannorris@chromium.org> |
| Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> |
| Reviewed-by: Ganapathi Bhat <gbhat@marvell.com> |
| Signed-off-by: Kalle Valo <kvalo@codeaurora.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/net/wireless/marvell/mwifiex/cfg80211.c | 2 +- |
| drivers/net/wireless/marvell/mwifiex/scan.c | 6 ++++++ |
| 2 files changed, 7 insertions(+), 1 deletion(-) |
| |
| --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c |
| +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c |
| @@ -4188,7 +4188,7 @@ int mwifiex_init_channel_scan_gap(struct |
| if (adapter->config_bands & BAND_A) |
| n_channels_a = mwifiex_band_5ghz.n_channels; |
| |
| - adapter->num_in_chan_stats = max_t(u32, n_channels_bg, n_channels_a); |
| + adapter->num_in_chan_stats = n_channels_bg + n_channels_a; |
| adapter->chan_stats = vmalloc(sizeof(*adapter->chan_stats) * |
| adapter->num_in_chan_stats); |
| |
| --- a/drivers/net/wireless/marvell/mwifiex/scan.c |
| +++ b/drivers/net/wireless/marvell/mwifiex/scan.c |
| @@ -2479,6 +2479,12 @@ mwifiex_update_chan_statistics(struct mw |
| sizeof(struct mwifiex_chan_stats); |
| |
| for (i = 0 ; i < num_chan; i++) { |
| + if (adapter->survey_idx >= adapter->num_in_chan_stats) { |
| + mwifiex_dbg(adapter, WARN, |
| + "FW reported too many channel results (max %d)\n", |
| + adapter->num_in_chan_stats); |
| + return; |
| + } |
| chan_stats.chan_num = fw_chan_stats->chan_num; |
| chan_stats.bandcfg = fw_chan_stats->bandcfg; |
| chan_stats.flags = fw_chan_stats->flags; |