From patchwork Wed Jan 4 11:09:41 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?UmFmYcWCIE1pxYJlY2tp?= X-Patchwork-Id: 9496471 X-Patchwork-Delegate: kvalo@adurom.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id DA1FB60413 for ; Wed, 4 Jan 2017 11:09:55 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D1FA627E71 for ; Wed, 4 Jan 2017 11:09:55 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C524127EED; Wed, 4 Jan 2017 11:09:55 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 385F627E71 for ; Wed, 4 Jan 2017 11:09:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966227AbdADLJx (ORCPT ); Wed, 4 Jan 2017 06:09:53 -0500 Received: from mail-lf0-f67.google.com ([209.85.215.67]:36066 "EHLO mail-lf0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966180AbdADLJw (ORCPT ); Wed, 4 Jan 2017 06:09:52 -0500 Received: by mail-lf0-f67.google.com with SMTP id j75so761014lfe.3 for ; Wed, 04 Jan 2017 03:09:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=9jvPTNoszWDPynlcOorOgQM0XoXrQgKvceWdYj3lHLk=; b=U/t2wQdGHIW6nS+Dij2SROB6EkC1v4jy8Iwa6oN3cGhf4vptnu3FIil8cFzRUmBagb mcrTjP3ACpSunWGD4+cidEUj5AXsh6zxzaSRJL+CQvvFrOy1GBVIHdRXGfEAafz4DjxD xrl2qS+LUi0W3PnFyIG4l00i2YtGIYutCdHt2UkaT0OmPp851yiy1fwY851TSHI8kaUQ nVitETniA+JpoDmiLpS06tE1UsMNXjVG3319C/TIhdd9k61z/94VJNO7co/i5wDX7g/s 1uQokHrHR00VOjrozMdhDKSnXTwpsLAGu2HFt4o8aezUCRsDyLx6QxqZ3rszfd9HA+nC tAvg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=9jvPTNoszWDPynlcOorOgQM0XoXrQgKvceWdYj3lHLk=; b=nFMmtUr2MIgFoaWmzJFCpc28ucyoW1m9yPnhY4uXBsC3McttFar9aCn/yDYVTAD9ik oJG8GgQQ/pWcjDA6OcIpiiy0/Q+DWD879cIwvtPdCRQwQn0BdIdGvQm2yx1fWq2aoOW7 S1sszfNatJUE5br6CatFzv0YmUVEcuk5ZRF0bYpMGnyBNJH/mfZUYYWxFIwnZ3P1mnZQ hlUx9KWzXXuDKDIptH2KmsmrvgjRx1hzabcCgdL7lHGC/gAbcyfXFpkzPWh88w6FRMU+ Am+AG4lvLcJDWpWKIuJkLmGD7LOjRjcZ9y4zVd8gLPnmwXwuIPKPdamkgDdtONQvrRNM B6nw== X-Gm-Message-State: AIkVDXKxTOnYwnloEd2gN4/rPI9cBgtIcxZxYrM1AHgucZF1/6hsnnKXffhnestRLQ17wg== X-Received: by 10.25.99.134 with SMTP id v6mr21345194lfi.170.1483528190234; Wed, 04 Jan 2017 03:09:50 -0800 (PST) Received: from linux-samsung.lan (ip-194-187-74-233.konfederacka.maverick.com.pl. [194.187.74.233]) by smtp.gmail.com with ESMTPSA id z26sm17411981lja.49.2017.01.04.03.09.48 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 04 Jan 2017 03:09:49 -0800 (PST) From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= To: Kalle Valo Cc: Arend van Spriel , Franky Lin , Hante Meuleman , Pieter-Paul Giesberts , Franky Lin , linux-wireless@vger.kernel.org, brcm80211-dev-list.pdl@broadcom.com, =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= Subject: [PATCH V3] brcmfmac: avoid writing channel out of allocated array Date: Wed, 4 Jan 2017 12:09:41 +0100 Message-Id: <20170104110941.21261-1-zajec5@gmail.com> X-Mailer: git-send-email 2.10.1 In-Reply-To: <20170103164930.29989-1-zajec5@gmail.com> References: <20170103164930.29989-1-zajec5@gmail.com> MIME-Version: 1.0 Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Rafał Miłecki 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. This never happened so far (we got a complete list of supported channels) but it means possible memory corruption so we should handle it anyway. This patch simply detects unexpected channel and ignores it. As we don't try to create new entry now, it's also safe to drop hw_value and center_freq assignment. For known channels we have these set anyway. I decided to fix this issue by assigning NULL or a target channel to the channel variable. This was one of possible ways, I prefefred this one as it also avoids using channel[index] over and over. Fixes: 58de92d2f95e ("brcmfmac: use static superset of channels for wiphy bands") Signed-off-by: Rafał Miłecki Acked-by: Arend van Spriel --- V2: Add extra comment in code for not-found channel. Make it clear this problem have never been seen so far Explain why it's safe to drop extra assignments Note & reason changing channel variable usage V3: Update error message as suggested by Arend. --- .../broadcom/brcm80211/brcmfmac/cfg80211.c | 32 ++++++++++++---------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c index 9c2c128..75ef23b 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,36 @@ 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) { + /* It seems firmware supports some channel we never + * considered. Something new in IEEE standard? + */ + brcmf_err("Ignoring unexpected firmware 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 +5909,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; } }