2021-03-04 12:37:58

by Ryder Lee

[permalink] [raw]
Subject: [PATCH 1/4] mt76: mt7915: fix rxrate reporting

Avoid directly updating sinfo->rxrate from firmware since rate_info might
be overwritten by wrong results even mt7915_mcu_get_rx_rate() fails check.

Add more error handlings accordingly.

Fixes: 11553d88d0b9 ("mt76: mt7915: query station rx rate from firmware")
Signed-off-by: Ryder Lee <[email protected]>
---
.../net/wireless/mediatek/mt76/mt7915/main.c | 5 +-
.../net/wireless/mediatek/mt76/mt7915/mcu.c | 47 ++++++++++---------
2 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/main.c b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
index cacc9b6933ca..bb49ea9307e0 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
@@ -839,9 +839,12 @@ static void mt7915_sta_statistics(struct ieee80211_hw *hw,
struct mt7915_phy *phy = mt7915_hw_phy(hw);
struct mt7915_sta *msta = (struct mt7915_sta *)sta->drv_priv;
struct mt7915_sta_stats *stats = &msta->stats;
+ struct rate_info rxrate = {};

- if (mt7915_mcu_get_rx_rate(phy, vif, sta, &sinfo->rxrate) == 0)
+ if (!mt7915_mcu_get_rx_rate(phy, vif, sta, &rxrate)) {
+ sinfo->rxrate = rxrate;
sinfo->filled |= BIT_ULL(NL80211_STA_INFO_RX_BITRATE);
+ }

if (!stats->tx_rate.legacy && !stats->tx_rate.flags)
return;
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
index e0c03b7f278d..b204c5dd7914 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
@@ -3552,9 +3552,8 @@ int mt7915_mcu_get_rx_rate(struct mt7915_phy *phy, struct ieee80211_vif *vif,
struct ieee80211_supported_band *sband;
struct mt7915_mcu_phy_rx_info *res;
struct sk_buff *skb;
- u16 flags = 0;
int ret;
- int i;
+ bool cck = false;

ret = mt76_mcu_send_and_get_msg(&dev->mt76, MCU_EXT_CMD(PHY_STAT_INFO),
&req, sizeof(req), true, &skb);
@@ -3568,48 +3567,53 @@ int mt7915_mcu_get_rx_rate(struct mt7915_phy *phy, struct ieee80211_vif *vif,

switch (res->mode) {
case MT_PHY_TYPE_CCK:
+ cck = true;
+ fallthrough;
case MT_PHY_TYPE_OFDM:
if (mphy->chandef.chan->band == NL80211_BAND_5GHZ)
sband = &mphy->sband_5g.sband;
else
sband = &mphy->sband_2g.sband;

- for (i = 0; i < sband->n_bitrates; i++) {
- if (rate->mcs != (sband->bitrates[i].hw_value & 0xf))
- continue;
-
- rate->legacy = sband->bitrates[i].bitrate;
- break;
- }
+ rate->mcs = mt76_get_rate(&dev->mt76, sband, rate->mcs, cck);
+ rate->legacy = sband->bitrates[rate->mcs].bitrate;
break;
case MT_PHY_TYPE_HT:
case MT_PHY_TYPE_HT_GF:
- if (rate->mcs > 31)
- return -EINVAL;
-
- flags |= RATE_INFO_FLAGS_MCS;
+ if (rate->mcs > 31) {
+ ret = -EINVAL;
+ goto out;
+ }

+ rate->flags = RATE_INFO_FLAGS_MCS;
if (res->gi)
- flags |= RATE_INFO_FLAGS_SHORT_GI;
+ rate->flags |= RATE_INFO_FLAGS_SHORT_GI;
break;
case MT_PHY_TYPE_VHT:
- flags |= RATE_INFO_FLAGS_VHT_MCS;
+ if (rate->mcs > 9) {
+ ret = -EINVAL;
+ goto out;
+ }

+ rate->flags = RATE_INFO_FLAGS_VHT_MCS;
if (res->gi)
- flags |= RATE_INFO_FLAGS_SHORT_GI;
+ rate->flags |= RATE_INFO_FLAGS_SHORT_GI;
break;
case MT_PHY_TYPE_HE_SU:
case MT_PHY_TYPE_HE_EXT_SU:
case MT_PHY_TYPE_HE_TB:
case MT_PHY_TYPE_HE_MU:
+ if (res->gi > NL80211_RATE_INFO_HE_GI_3_2 || rate->mcs > 11) {
+ ret = -EINVAL;
+ goto out;
+ }
rate->he_gi = res->gi;
-
- flags |= RATE_INFO_FLAGS_HE_MCS;
+ rate->flags = RATE_INFO_FLAGS_HE_MCS;
break;
default:
- break;
+ ret = -EINVAL;
+ goto out;
}
- rate->flags = flags;

switch (res->bw) {
case IEEE80211_STA_RX_BW_160:
@@ -3626,7 +3630,8 @@ int mt7915_mcu_get_rx_rate(struct mt7915_phy *phy, struct ieee80211_vif *vif,
break;
}

+out:
dev_kfree_skb(skb);

- return 0;
+ return ret;
}
--
2.18.0


2021-03-04 12:37:58

by Ryder Lee

[permalink] [raw]
Subject: [PATCH 3/4] mt76: mt7915: check mcu returned values in mt7915_ops

Properly check returned values from mcu utility routines in
mt7915_ops.

Signed-off-by: Ryder Lee <[email protected]>
---
.../net/wireless/mediatek/mt76/mt7915/main.c | 58 ++++++++++++++-----
1 file changed, 42 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/main.c b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
index bb49ea9307e0..255f87aa1830 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
@@ -25,6 +25,7 @@ static int mt7915_start(struct ieee80211_hw *hw)
struct mt7915_dev *dev = mt7915_hw_dev(hw);
struct mt7915_phy *phy = mt7915_hw_phy(hw);
bool running;
+ int ret;

flush_work(&dev->init_work);

@@ -33,21 +34,44 @@ static int mt7915_start(struct ieee80211_hw *hw)
running = mt7915_dev_running(dev);

if (!running) {
- mt7915_mcu_set_pm(dev, 0, 0);
- mt7915_mcu_set_mac(dev, 0, true, true);
- mt7915_mcu_set_scs(dev, 0, true);
+ ret = mt7915_mcu_set_pm(dev, 0, 0);
+ if (ret)
+ goto out;
+
+ ret = mt7915_mcu_set_mac(dev, 0, true, true);
+ if (ret)
+ goto out;
+
+ ret = mt7915_mcu_set_scs(dev, 0, true);
+ if (ret)
+ goto out;
+
mt7915_mac_enable_nf(dev, 0);
}

if (phy != &dev->phy) {
- mt7915_mcu_set_pm(dev, 1, 0);
- mt7915_mcu_set_mac(dev, 1, true, true);
- mt7915_mcu_set_scs(dev, 1, true);
+ ret = mt7915_mcu_set_pm(dev, 1, 0);
+ if (ret)
+ goto out;
+
+ ret = mt7915_mcu_set_mac(dev, 1, true, true);
+ if (ret)
+ goto out;
+
+ ret = mt7915_mcu_set_scs(dev, 1, true);
+ if (ret)
+ goto out;
+
mt7915_mac_enable_nf(dev, 1);
}

- mt7915_mcu_set_sku_en(phy, true);
- mt7915_mcu_set_chan_info(phy, MCU_EXT_CMD(SET_RX_PATH));
+ ret = mt7915_mcu_set_sku_en(phy, true);
+ if (ret)
+ goto out;
+
+ ret = mt7915_mcu_set_chan_info(phy, MCU_EXT_CMD(SET_RX_PATH));
+ if (ret)
+ goto out;

set_bit(MT76_STATE_RUNNING, &phy->mt76->state);

@@ -58,9 +82,10 @@ static int mt7915_start(struct ieee80211_hw *hw)
if (!running)
mt7915_mac_reset_counters(phy);

+out:
mutex_unlock(&dev->mt76.mutex);

- return 0;
+ return ret;
}

static void mt7915_stop(struct ieee80211_hw *hw)
@@ -631,12 +656,13 @@ static int mt7915_set_rts_threshold(struct ieee80211_hw *hw, u32 val)
{
struct mt7915_dev *dev = mt7915_hw_dev(hw);
struct mt7915_phy *phy = mt7915_hw_phy(hw);
+ int ret;

mutex_lock(&dev->mt76.mutex);
- mt7915_mcu_set_rts_thresh(phy, val);
+ ret = mt7915_mcu_set_rts_thresh(phy, val);
mutex_unlock(&dev->mt76.mutex);

- return 0;
+ return ret;
}

static int
@@ -663,22 +689,22 @@ mt7915_ampdu_action(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
case IEEE80211_AMPDU_RX_START:
mt76_rx_aggr_start(&dev->mt76, &msta->wcid, tid, ssn,
params->buf_size);
- mt7915_mcu_add_rx_ba(dev, params, true);
+ ret = mt7915_mcu_add_rx_ba(dev, params, true);
break;
case IEEE80211_AMPDU_RX_STOP:
mt76_rx_aggr_stop(&dev->mt76, &msta->wcid, tid);
- mt7915_mcu_add_rx_ba(dev, params, false);
+ ret = mt7915_mcu_add_rx_ba(dev, params, false);
break;
case IEEE80211_AMPDU_TX_OPERATIONAL:
mtxq->aggr = true;
mtxq->send_bar = false;
- mt7915_mcu_add_tx_ba(dev, params, true);
+ ret = mt7915_mcu_add_tx_ba(dev, params, true);
break;
case IEEE80211_AMPDU_TX_STOP_FLUSH:
case IEEE80211_AMPDU_TX_STOP_FLUSH_CONT:
mtxq->aggr = false;
clear_bit(tid, &msta->ampdu_state);
- mt7915_mcu_add_tx_ba(dev, params, false);
+ ret = mt7915_mcu_add_tx_ba(dev, params, false);
break;
case IEEE80211_AMPDU_TX_START:
set_bit(tid, &msta->ampdu_state);
@@ -687,7 +713,7 @@ mt7915_ampdu_action(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
case IEEE80211_AMPDU_TX_STOP_CONT:
mtxq->aggr = false;
clear_bit(tid, &msta->ampdu_state);
- mt7915_mcu_add_tx_ba(dev, params, false);
+ ret = mt7915_mcu_add_tx_ba(dev, params, false);
ieee80211_stop_tx_ba_cb_irqsafe(vif, sta->addr, tid);
break;
}
--
2.18.0

2021-03-04 12:38:00

by Ryder Lee

[permalink] [raw]
Subject: [PATCH 2/4] mt76: mt7915: fix txrate reporting

Properly check rate_info to fix unexpected reporting.

[ 1215.161863] Call trace:
[ 1215.164307] cfg80211_calculate_bitrate+0x124/0x200 [cfg80211]
[ 1215.170139] ieee80211s_update_metric+0x80/0xc0 [mac80211]
[ 1215.175624] ieee80211_tx_status_ext+0x508/0x838 [mac80211]
[ 1215.181190] mt7915_mcu_get_rx_rate+0x28c/0x8d0 [mt7915e]
[ 1215.186580] mt7915_mac_tx_free+0x324/0x7c0 [mt7915e]
[ 1215.191623] mt7915_queue_rx_skb+0xa8/0xd0 [mt7915e]
[ 1215.196582] mt76_dma_cleanup+0x7b0/0x11d0 [mt76]
[ 1215.201276] __napi_poll+0x38/0xf8
[ 1215.204668] napi_workfn+0x40/0x80
[ 1215.208062] process_one_work+0x1fc/0x390
[ 1215.212062] worker_thread+0x48/0x4d0
[ 1215.215715] kthread+0x120/0x128
[ 1215.218935] ret_from_fork+0x10/0x1c

Fixes: e57b7901469f ("mt76: add mac80211 driver for MT7915 PCIe-based chipsets")
Fixes: e4c5ead632ff ("mt76: mt7915: rename mt7915_mcu_get_rate_info to mt7915_mcu_get_tx_rate")
Reported-by: Evelyn Tsai <[email protected]>
Signed-off-by: Ryder Lee <[email protected]>
---
.../net/wireless/mediatek/mt76/mt7915/mcu.c | 38 ++++++++++++-------
1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
index b204c5dd7914..edb58f6f2640 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
@@ -364,54 +364,62 @@ mt7915_mcu_rx_radar_detected(struct mt7915_dev *dev, struct sk_buff *skb)
dev->hw_pattern++;
}

-static void
+static int
mt7915_mcu_tx_rate_parse(struct mt76_phy *mphy, struct mt7915_mcu_ra_info *ra,
struct rate_info *rate, u16 r)
{
struct ieee80211_supported_band *sband;
u16 ru_idx = le16_to_cpu(ra->ru_idx);
- u16 flags = 0;
+ bool cck = false;

rate->mcs = FIELD_GET(MT_RA_RATE_MCS, r);
rate->nss = FIELD_GET(MT_RA_RATE_NSS, r) + 1;

switch (FIELD_GET(MT_RA_RATE_TX_MODE, r)) {
case MT_PHY_TYPE_CCK:
+ cck = true;
+ fallthrough;
case MT_PHY_TYPE_OFDM:
if (mphy->chandef.chan->band == NL80211_BAND_5GHZ)
sband = &mphy->sband_5g.sband;
else
sband = &mphy->sband_2g.sband;

+ rate->mcs = mt76_get_rate(mphy->dev, sband, rate->mcs, cck);
rate->legacy = sband->bitrates[rate->mcs].bitrate;
break;
case MT_PHY_TYPE_HT:
case MT_PHY_TYPE_HT_GF:
rate->mcs += (rate->nss - 1) * 8;
- flags |= RATE_INFO_FLAGS_MCS;
+ if (rate->mcs > 31)
+ return -EINVAL;

+ rate->flags = RATE_INFO_FLAGS_MCS;
if (ra->gi)
- flags |= RATE_INFO_FLAGS_SHORT_GI;
+ rate->flags |= RATE_INFO_FLAGS_SHORT_GI;
break;
case MT_PHY_TYPE_VHT:
- flags |= RATE_INFO_FLAGS_VHT_MCS;
+ if (rate->mcs > 9)
+ return -EINVAL;

+ rate->flags = RATE_INFO_FLAGS_VHT_MCS;
if (ra->gi)
- flags |= RATE_INFO_FLAGS_SHORT_GI;
+ rate->flags |= RATE_INFO_FLAGS_SHORT_GI;
break;
case MT_PHY_TYPE_HE_SU:
case MT_PHY_TYPE_HE_EXT_SU:
case MT_PHY_TYPE_HE_TB:
case MT_PHY_TYPE_HE_MU:
+ if (ra->gi > NL80211_RATE_INFO_HE_GI_3_2 || rate->mcs > 11)
+ return -EINVAL;
+
rate->he_gi = ra->gi;
rate->he_dcm = FIELD_GET(MT_RA_RATE_DCM_EN, r);
-
- flags |= RATE_INFO_FLAGS_HE_MCS;
+ rate->flags = RATE_INFO_FLAGS_HE_MCS;
break;
default:
- break;
+ return -EINVAL;
}
- rate->flags = flags;

if (ru_idx) {
switch (ru_idx) {
@@ -448,6 +456,8 @@ mt7915_mcu_tx_rate_parse(struct mt76_phy *mphy, struct mt7915_mcu_ra_info *ra,
break;
}
}
+
+ return 0;
}

static void
@@ -478,12 +488,12 @@ mt7915_mcu_tx_rate_report(struct mt7915_dev *dev, struct sk_buff *skb)
mphy = dev->mt76.phy2;

/* current rate */
- mt7915_mcu_tx_rate_parse(mphy, ra, &rate, curr);
- stats->tx_rate = rate;
+ if(!mt7915_mcu_tx_rate_parse(mphy, ra, &rate, curr))
+ stats->tx_rate = rate;

/* probing rate */
- mt7915_mcu_tx_rate_parse(mphy, ra, &prob_rate, probe);
- stats->prob_rate = prob_rate;
+ if(!mt7915_mcu_tx_rate_parse(mphy, ra, &prob_rate, probe))
+ stats->prob_rate = prob_rate;

if (attempts) {
u16 success = le16_to_cpu(ra->success);
--
2.18.0

2021-03-04 12:38:55

by Ryder Lee

[permalink] [raw]
Subject: [PATCH 4/4] mt76: mt7615: check mcu returned values in mt7615_ops

Properly check returned values from mcu utility routines in
mt7615_ops.

Signed-off-by: Ryder Lee <[email protected]>
---
.../net/wireless/mediatek/mt76/mt7615/main.c | 54 +++++++++++++------
1 file changed, 37 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/main.c b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
index 68accb37ea28..1564bcdde2d3 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
@@ -29,6 +29,7 @@ static int mt7615_start(struct ieee80211_hw *hw)
struct mt7615_dev *dev = mt7615_hw_dev(hw);
struct mt7615_phy *phy = mt7615_hw_phy(hw);
bool running;
+ int ret;

if (!mt7615_wait_for_mcu_init(dev))
return -EIO;
@@ -38,21 +39,38 @@ static int mt7615_start(struct ieee80211_hw *hw)
running = mt7615_dev_running(dev);

if (!running) {
- mt7615_mcu_set_pm(dev, 0, 0);
- mt76_connac_mcu_set_mac_enable(&dev->mt76, 0, true, false);
+ ret = mt7615_mcu_set_pm(dev, 0, 0);
+ if (ret)
+ goto out;
+
+ ret = mt76_connac_mcu_set_mac_enable(&dev->mt76, 0, true, false);
+ if (ret)
+ goto out;
+
mt7615_mac_enable_nf(dev, 0);
}

if (phy != &dev->phy) {
- mt7615_mcu_set_pm(dev, 1, 0);
- mt76_connac_mcu_set_mac_enable(&dev->mt76, 1, true, false);
+ ret = mt7615_mcu_set_pm(dev, 1, 0);
+ if (ret)
+ goto out;
+
+ ret = mt76_connac_mcu_set_mac_enable(&dev->mt76, 1, true, false);
+ if (ret)
+ goto out;
+
mt7615_mac_enable_nf(dev, 1);
}

- if (mt7615_firmware_offload(dev))
- mt76_connac_mcu_set_channel_domain(phy->mt76);
+ if (mt7615_firmware_offload(dev)) {
+ ret = mt76_connac_mcu_set_channel_domain(phy->mt76);
+ if (ret)
+ goto out;
+ }

- mt7615_mcu_set_chan_info(phy, MCU_EXT_CMD_SET_RX_PATH);
+ ret = mt7615_mcu_set_chan_info(phy, MCU_EXT_CMD_SET_RX_PATH);
+ if (ret)
+ goto out;

set_bit(MT76_STATE_RUNNING, &phy->mt76->state);

@@ -62,6 +80,7 @@ static int mt7615_start(struct ieee80211_hw *hw)
if (!running)
mt7615_mac_reset_counters(dev);

+out:
mt7615_mutex_release(dev);

return 0;
@@ -715,13 +734,13 @@ static int mt7615_set_rts_threshold(struct ieee80211_hw *hw, u32 val)
{
struct mt7615_dev *dev = mt7615_hw_dev(hw);
struct mt7615_phy *phy = mt7615_hw_phy(hw);
- int band = phy != &dev->phy;
+ int err, band = phy != &dev->phy;

mt7615_mutex_acquire(dev);
- mt76_connac_mcu_set_rts_thresh(&dev->mt76, val, band);
+ err = mt76_connac_mcu_set_rts_thresh(&dev->mt76, val, band);
mt7615_mutex_release(dev);

- return 0;
+ return err;
}

static int
@@ -749,16 +768,16 @@ mt7615_ampdu_action(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
case IEEE80211_AMPDU_RX_START:
mt76_rx_aggr_start(&dev->mt76, &msta->wcid, tid, ssn,
params->buf_size);
- mt7615_mcu_add_rx_ba(dev, params, true);
+ ret = mt7615_mcu_add_rx_ba(dev, params, true);
break;
case IEEE80211_AMPDU_RX_STOP:
mt76_rx_aggr_stop(&dev->mt76, &msta->wcid, tid);
- mt7615_mcu_add_rx_ba(dev, params, false);
+ ret = mt7615_mcu_add_rx_ba(dev, params, false);
break;
case IEEE80211_AMPDU_TX_OPERATIONAL:
mtxq->aggr = true;
mtxq->send_bar = false;
- mt7615_mcu_add_tx_ba(dev, params, true);
+ ret = mt7615_mcu_add_tx_ba(dev, params, true);
ssn = mt7615_mac_get_sta_tid_sn(dev, msta->wcid.idx, tid);
ieee80211_send_bar(vif, sta->addr, tid,
IEEE80211_SN_TO_SEQ(ssn));
@@ -766,7 +785,7 @@ mt7615_ampdu_action(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
case IEEE80211_AMPDU_TX_STOP_FLUSH:
case IEEE80211_AMPDU_TX_STOP_FLUSH_CONT:
mtxq->aggr = false;
- mt7615_mcu_add_tx_ba(dev, params, false);
+ ret = mt7615_mcu_add_tx_ba(dev, params, false);
break;
case IEEE80211_AMPDU_TX_START:
ssn = mt7615_mac_get_sta_tid_sn(dev, msta->wcid.idx, tid);
@@ -775,7 +794,7 @@ mt7615_ampdu_action(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
break;
case IEEE80211_AMPDU_TX_STOP_CONT:
mtxq->aggr = false;
- mt7615_mcu_add_tx_ba(dev, params, false);
+ ret = mt7615_mcu_add_tx_ba(dev, params, false);
ieee80211_stop_tx_ba_cb_irqsafe(vif, sta->addr, tid);
break;
}
@@ -1085,6 +1104,7 @@ static int mt7615_cancel_remain_on_channel(struct ieee80211_hw *hw,
struct ieee80211_vif *vif)
{
struct mt7615_phy *phy = mt7615_hw_phy(hw);
+ int err;

if (!test_and_clear_bit(MT76_STATE_ROC, &phy->mt76->state))
return 0;
@@ -1093,10 +1113,10 @@ static int mt7615_cancel_remain_on_channel(struct ieee80211_hw *hw,
cancel_work_sync(&phy->roc_work);

mt7615_mutex_acquire(phy->dev);
- mt7615_mcu_set_roc(phy, vif, NULL, 0);
+ err = mt7615_mcu_set_roc(phy, vif, NULL, 0);
mt7615_mutex_release(phy->dev);

- return 0;
+ return err;
}

static void mt7615_sta_set_decap_offload(struct ieee80211_hw *hw,
--
2.18.0