All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Rafał Miłecki" <zajec5@gmail.com>
To: Kalle Valo <kvalo@codeaurora.org>
Cc: "Arend van Spriel" <arend.vanspriel@broadcom.com>,
	"Franky Lin" <franky.lin@broadcom.com>,
	"Hante Meuleman" <hante.meuleman@broadcom.com>,
	"Pieter-Paul Giesberts" <pieter-paul.giesberts@broadcom.com>,
	"Franky Lin" <frankyl@broadcom.com>,
	linux-wireless@vger.kernel.org,
	brcm80211-dev-list.pdl@broadcom.com,
	"Rafał Miłecki" <rafal@milecki.pl>
Subject: [PATCH] brcmfmac: avoid writing channel out of allocated array
Date: Tue,  3 Jan 2017 09:38:58 +0100	[thread overview]
Message-ID: <20170103083858.6981-1-zajec5@gmail.com> (raw)

From: Rafał Miłecki <rafal@milecki.pl>

Our code was assigning number of channels to the index variable by
default. If firmware reported channel we didn't predict this would
result in using that initial index value and writing out of array.

Fix this by detecting unexpected channel and ignoring it.

Fixes: 58de92d2f95e ("brcmfmac: use static superset of channels for wiphy bands")
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
I'm not sure what kind of material it is. It fixes possible memory corruption
(serious thing?) but this bug was there since Apr 2015, so is it worth fixing
in 4.10? Or maybe I should even cc stable?
I don't think any released firmware reports any unexpected channel, so I guess
noone ever hit this problem. I just noticed this possible problem when working
on another feature.
---
 .../broadcom/brcm80211/brcmfmac/cfg80211.c         | 29 +++++++++++-----------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 13ca3eb..0babfc7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5825,7 +5825,6 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
 	u32 i, j;
 	u32 total;
 	u32 chaninfo;
-	u32 index;
 
 	pbuf = kzalloc(BRCMF_DCMD_MEDLEN, GFP_KERNEL);
 
@@ -5873,33 +5872,33 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
 		    ch.bw == BRCMU_CHAN_BW_80)
 			continue;
 
-		channel = band->channels;
-		index = band->n_channels;
+		channel = NULL;
 		for (j = 0; j < band->n_channels; j++) {
-			if (channel[j].hw_value == ch.control_ch_num) {
-				index = j;
+			if (band->channels[j].hw_value == ch.control_ch_num) {
+				channel = &band->channels[j];
 				break;
 			}
 		}
-		channel[index].center_freq =
-			ieee80211_channel_to_frequency(ch.control_ch_num,
-						       band->band);
-		channel[index].hw_value = ch.control_ch_num;
+		if (!channel) {
+			brcmf_err("Firmware reported unexpected channel %d\n",
+				  ch.control_ch_num);
+			continue;
+		}
 
 		/* assuming the chanspecs order is HT20,
 		 * HT40 upper, HT40 lower, and VHT80.
 		 */
 		if (ch.bw == BRCMU_CHAN_BW_80) {
-			channel[index].flags &= ~IEEE80211_CHAN_NO_80MHZ;
+			channel->flags &= ~IEEE80211_CHAN_NO_80MHZ;
 		} else if (ch.bw == BRCMU_CHAN_BW_40) {
-			brcmf_update_bw40_channel_flag(&channel[index], &ch);
+			brcmf_update_bw40_channel_flag(channel, &ch);
 		} else {
 			/* enable the channel and disable other bandwidths
 			 * for now as mentioned order assure they are enabled
 			 * for subsequent chanspecs.
 			 */
-			channel[index].flags = IEEE80211_CHAN_NO_HT40 |
-					       IEEE80211_CHAN_NO_80MHZ;
+			channel->flags = IEEE80211_CHAN_NO_HT40 |
+					 IEEE80211_CHAN_NO_80MHZ;
 			ch.bw = BRCMU_CHAN_BW_20;
 			cfg->d11inf.encchspec(&ch);
 			chaninfo = ch.chspec;
@@ -5907,11 +5906,11 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
 						       &chaninfo);
 			if (!err) {
 				if (chaninfo & WL_CHAN_RADAR)
-					channel[index].flags |=
+					channel->flags |=
 						(IEEE80211_CHAN_RADAR |
 						 IEEE80211_CHAN_NO_IR);
 				if (chaninfo & WL_CHAN_PASSIVE)
-					channel[index].flags |=
+					channel->flags |=
 						IEEE80211_CHAN_NO_IR;
 			}
 		}
-- 
2.10.1

             reply	other threads:[~2017-01-03  8:39 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-03  8:38 Rafał Miłecki [this message]
2017-01-03 11:02 ` [PATCH] brcmfmac: avoid writing channel out of allocated array Arend Van Spriel
2017-01-03 11:31   ` Rafał Miłecki
2017-01-03 13:19     ` Arend Van Spriel
2017-01-03 14:14       ` Rafał Miłecki
2017-01-03 15:49         ` Rafał Miłecki
2017-01-04  8:12           ` Kalle Valo
2017-01-04  8:14     ` Kalle Valo
2017-01-04  8:08   ` Kalle Valo
2017-01-03 16:49 ` [PATCH next V2] " Rafał Miłecki
2017-01-04  9:39   ` Arend Van Spriel
2017-01-04 10:40     ` Rafał Miłecki
2017-01-04 10:48       ` Arend Van Spriel
2017-01-04 11:04         ` Rafał Miłecki
2017-01-04 11:09   ` [PATCH V3] " Rafał Miłecki
2017-01-04 11:11     ` Arend Van Spriel
2017-01-17 11:57     ` [V3] " Kalle Valo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170103083858.6981-1-zajec5@gmail.com \
    --to=zajec5@gmail.com \
    --cc=arend.vanspriel@broadcom.com \
    --cc=brcm80211-dev-list.pdl@broadcom.com \
    --cc=franky.lin@broadcom.com \
    --cc=frankyl@broadcom.com \
    --cc=hante.meuleman@broadcom.com \
    --cc=kvalo@codeaurora.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=pieter-paul.giesberts@broadcom.com \
    --cc=rafal@milecki.pl \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.