public inbox for ath12k@lists.infradead.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Pradeep Kumar Chitrapu <quic_pradeepc@quicinc.com>,
	Roopni Devanathan <quic_rdevanat@quicinc.com>,
	Vasanthakumar Thiagarajan
	<vasanthakumar.thiagarajan@oss.qualcomm.com>,
	Ping-Ke Shih <pkshih@realtek.com>,
	Jeff Johnson <jeff.johnson@oss.qualcomm.com>,
	Sasha Levin <sashal@kernel.org>,
	jjohnson@kernel.org, linux-wireless@vger.kernel.org,
	ath12k@lists.infradead.org
Subject: [PATCH AUTOSEL 6.14 101/108] wifi: ath12k: Fix incorrect rates sent to firmware
Date: Tue,  3 Jun 2025 20:55:24 -0400	[thread overview]
Message-ID: <20250604005531.4178547-101-sashal@kernel.org> (raw)
In-Reply-To: <20250604005531.4178547-1-sashal@kernel.org>

From: Pradeep Kumar Chitrapu <quic_pradeepc@quicinc.com>

[ Upstream commit cb1790249361ba9396b06b1af2500147e6e42e5e ]

Before firmware assert, if there is a station interface in the device
which is not associated with an AP, the basic rates are set to zero.
Following this, during firmware recovery, when basic rates are zero,
ath12k driver is sending invalid rate codes, which are negative values,
to firmware. This results in firmware assert.

Fix this by checking if rate codes are valid, before sending them
to the firmware.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.4.1-00199-QCAHKSWPL_SILICONZ-1

Signed-off-by: Pradeep Kumar Chitrapu <quic_pradeepc@quicinc.com>
Signed-off-by: Roopni Devanathan <quic_rdevanat@quicinc.com>
Reviewed-by: Vasanthakumar Thiagarajan <vasanthakumar.thiagarajan@oss.qualcomm.com>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
Link: https://patch.msgid.link/20250320112426.1956961-1-quic_rdevanat@quicinc.com
Signed-off-by: Jeff Johnson <jeff.johnson@oss.qualcomm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

**YES** This commit should be backported to stable kernel trees. ##
Analysis This commit fixes a critical bug that causes firmware crashes
during firmware recovery scenarios, making it an important candidate for
stable backporting. ### Issue Analysis **The Problem:** The commit
addresses a scenario where during firmware recovery, when a station
interface exists but is not associated with an AP, the `basic_rates`
field becomes zero. The original code uses `ffs(bss_conf->basic_rates) -
1` to calculate the basic rate index, which produces a negative value
(-1) when `basic_rates` is zero, since `ffs(0)` returns 0. **Code
Changes Analysis:** 1. **In `ath12k_recalculate_mgmt_rate()`:** ```c -
basic_rate_idx = ffs(bss_conf->basic_rates) - 1; + if
(bss_conf->basic_rates) + basic_rate_idx = __ffs(bss_conf->basic_rates);
+ else + basic_rate_idx = 0; ``` 2. **In
`ath12k_mac_bss_info_changed()`:** ```c - if (mcast_rate > 0) + if
(mcast_rate > 0) { rateidx = mcast_rate - 1; - else - rateidx =
ffs(info->basic_rates) - 1; + } else { + if (info->basic_rates) +
rateidx = __ffs(info->basic_rates); + else + rateidx = 0; + } ``` ###
Why This Should Be Backported **1. Firmware Crash Prevention:** The
commit directly prevents firmware crashes by ensuring invalid rate codes
(negative values) are never sent to firmware. This is a critical
stability fix. **2. Small and Contained Fix:** The changes are minimal
and focused - just adding proper validation for the `basic_rates` field
before using it in rate calculations. The fix doesn't introduce new
features or architectural changes. **3. Common Scenario Impact:** The
issue occurs during firmware recovery when station interfaces exist but
aren't associated - a scenario that can happen during normal operation,
especially in environments with frequent disconnections or recovery
events. **4. Consistent with Stable Tree Criteria:** - Fixes an
important bug affecting users (firmware crashes) - Changes are minimal
and low-risk - No new functionality introduced - Confined to the ath12k
subsystem **5. Similarity to Backported Commits:** This fix follows the
same pattern as the historical examples that were backported (marked
"YES"): - Similar Commit #1: Fixed incorrect multicast/broadcast rate
setting that caused broken mesh paths - Similar Commit #3: Fixed invalid
AMPDU factor calculation - Similar Commit #4: Fixed failed frame
transmission handling to prevent metric calculation issues - Similar
Commit #5: Fixed firmware crash due to invalid peer NSS values **6. Low
Regression Risk:** The fix uses safe fallback behavior (defaulting to
rate index 0) when `basic_rates` is zero, which is much safer than
sending negative rate codes to firmware. The commit addresses a clear
bug that causes firmware stability issues in a common operational
scenario, with a minimal and safe fix that follows established patterns
for stable tree inclusion.

 drivers/net/wireless/ath/ath12k/mac.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 06b2baccd1335..b06836525e2c4 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -3413,7 +3413,10 @@ static void ath12k_recalculate_mgmt_rate(struct ath12k *ar,
 	}
 
 	sband = hw->wiphy->bands[def->chan->band];
-	basic_rate_idx = ffs(bss_conf->basic_rates) - 1;
+	if (bss_conf->basic_rates)
+		basic_rate_idx = __ffs(bss_conf->basic_rates);
+	else
+		basic_rate_idx = 0;
 	bitrate = sband->bitrates[basic_rate_idx].bitrate;
 
 	hw_rate_code = ath12k_mac_get_rate_hw_value(bitrate);
@@ -3811,10 +3814,14 @@ static void ath12k_mac_bss_info_changed(struct ath12k *ar,
 		band = def.chan->band;
 		mcast_rate = info->mcast_rate[band];
 
-		if (mcast_rate > 0)
+		if (mcast_rate > 0) {
 			rateidx = mcast_rate - 1;
-		else
-			rateidx = ffs(info->basic_rates) - 1;
+		} else {
+			if (info->basic_rates)
+				rateidx = __ffs(info->basic_rates);
+			else
+				rateidx = 0;
+		}
 
 		if (ar->pdev->cap.supported_bands & WMI_HOST_WLAN_5G_CAP)
 			rateidx += ATH12K_MAC_FIRST_OFDM_RATE_IDX;
-- 
2.39.5



  parent reply	other threads:[~2025-06-04  2:01 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20250604005531.4178547-1-sashal@kernel.org>
2025-06-04  0:54 ` [PATCH AUTOSEL 6.14 027/108] wifi: ath12k: fix macro definition HAL_RX_MSDU_PKT_LENGTH_GET Sasha Levin
2025-06-04  0:54 ` [PATCH AUTOSEL 6.14 028/108] wifi: ath12k: fix a possible dead lock caused by ab->base_lock Sasha Levin
2025-06-04  0:55 ` [PATCH AUTOSEL 6.14 082/108] wifi: ath12k: correctly handle mcast packets for clients Sasha Levin
2025-06-04  0:55 ` [PATCH AUTOSEL 6.14 083/108] wifi: ath12k: using msdu end descriptor to check for rx multicast packets Sasha Levin
2025-06-04  0:55 ` [PATCH AUTOSEL 6.14 086/108] wifi: ath12k: make assoc link associate first Sasha Levin
2025-06-04  0:55 ` [PATCH AUTOSEL 6.14 089/108] wifi: ath12k: fix failed to set mhi state error during reboot with hardware grouping Sasha Levin
2025-06-04  0:55 ` Sasha Levin [this message]
2025-06-04  0:55 ` [PATCH AUTOSEL 6.14 102/108] wifi: ath12k: Fix the enabling of REO queue lookup table feature Sasha Levin
2025-06-04  0:55 ` [PATCH AUTOSEL 6.14 103/108] wifi: ath12k: Fix memory leak due to multiple rx_stats allocation Sasha Levin
2025-06-04  0:55 ` [PATCH AUTOSEL 6.14 105/108] wifi: ath12k: fix link valid field initialization in the monitor Rx Sasha Levin
2025-06-04  0:55 ` [PATCH AUTOSEL 6.14 106/108] wifi: ath12k: fix incorrect CE addresses Sasha Levin
2025-06-04  0:55 ` [PATCH AUTOSEL 6.14 107/108] wifi: ath12k: Pass correct values of center freq1 and center freq2 for 160 MHz Sasha Levin

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=20250604005531.4178547-101-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=ath12k@lists.infradead.org \
    --cc=jeff.johnson@oss.qualcomm.com \
    --cc=jjohnson@kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=pkshih@realtek.com \
    --cc=quic_pradeepc@quicinc.com \
    --cc=quic_rdevanat@quicinc.com \
    --cc=stable@vger.kernel.org \
    --cc=vasanthakumar.thiagarajan@oss.qualcomm.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox