* [PATCH AUTOSEL 6.18-5.10] wifi: cfg80211: Fix bitrate calculation overflow for HE rates
[not found] <20260128223332.2806589-1-sashal@kernel.org>
@ 2026-01-28 22:33 ` Sasha Levin
0 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-01-28 22:33 UTC (permalink / raw)
To: patches, stable
Cc: Veerendranath Jakkam, Johannes Berg, Sasha Levin, johannes,
linux-wireless
From: Veerendranath Jakkam <veerendranath.jakkam@oss.qualcomm.com>
[ Upstream commit a3034bf0746d88a00cceda9541534a5721445a24 ]
An integer overflow occurs in cfg80211_calculate_bitrate_he() when
calculating bitrates for high throughput HE configurations.
For example, with 160 MHz bandwidth, HE-MCS 13, HE-NSS 4, and HE-GI 0,
the multiplication (result * rate->nss) overflows the 32-bit 'result'
variable before division by 8, leading to significantly underestimated
bitrate values.
The overflow occurs because the NSS multiplication operates on a 32-bit
integer that cannot accommodate intermediate values exceeding
4,294,967,295. When overflow happens, the value wraps around, producing
incorrect bitrates for high MCS and NSS combinations.
Fix this by utilizing the 64-bit 'tmp' variable for the NSS
multiplication and subsequent divisions via do_div(). This approach
preserves full precision throughout the entire calculation, with the
final value assigned to 'result' only after completing all operations.
Signed-off-by: Veerendranath Jakkam <veerendranath.jakkam@oss.qualcomm.com>
Link: https://patch.msgid.link/20260109-he_bitrate_overflow-v1-1-95575e466b6e@oss.qualcomm.com
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
I can see the buggy code is still present in this version (the fix
hasn't been applied yet to this version of util.c). This is the code
being patched. Let me double check the diff shows exactly what I expect:
Looking at the diff again:
```c
- result = tmp;
/* and take NSS, DCM into account */
- result = (result * rate->nss) / 8;
+ tmp *= rate->nss;
+ do_div(tmp, 8);
if (rate->he_dcm)
- result /= 2;
+ do_div(tmp, 2);
+
+ result = tmp;
```
The fix:
1. Removes `result = tmp` after the MCS division (keeps value in 64-bit
`tmp`)
2. Multiplies NSS in 64-bit: `tmp *= rate->nss` instead of 32-bit
`result * rate->nss`
3. Uses `do_div(tmp, 8)` instead of 32-bit division
4. Uses `do_div(tmp, 2)` for DCM instead of 32-bit division
5. Only assigns to 32-bit `result` at the end when the value is
guaranteed to fit
This matches exactly the same fix pattern that was applied to
`cfg80211_calculate_bitrate_eht()` in commit 18429c51c7ff6.
## Summary Analysis
### 1. COMMIT MESSAGE ANALYSIS
- **Subject**: Clearly indicates a bug fix for bitrate calculation
overflow in HE rates
- **Body**: Provides detailed technical explanation of the overflow
scenario (160 MHz, MCS 13, NSS 4, GI 0)
- **Root cause**: 32-bit integer overflow when multiplying `result *
rate->nss` before division by 8
- **Fix approach**: Use 64-bit `tmp` variable throughout the
calculation, assign to 32-bit `result` only at the end
- **Tags**: Has Signed-off-by from both Qualcomm developer and Johannes
Berg (wireless maintainer)
### 2. CODE CHANGE ANALYSIS
- **Bug mechanism**: When `result` (a u32) is large (e.g., 0x44b85dd4 =
1,152,933,332), multiplying by NSS 4 produces 0x112e17750
(4,611,733,328) which overflows u32 max (0xFFFFFFFF = 4,294,967,295)
- **Result of bug**: The overflow wraps to 0x12e17750, producing a
bitrate of ~3,959 (400 Mbps) instead of correct ~57,646 (5.76 Gbps)
- **Fix correctness**: The fix keeps all arithmetic in 64-bit until the
final assignment, preventing overflow
- **DCM handling**: Also moved to 64-bit for consistency and safety
### 3. CLASSIFICATION
- **Type**: Bug fix (integer overflow causing incorrect bitrate
reporting)
- **Security**: Not a security issue, but affects user-visible data
correctness
- **Exception categories**: Not applicable - this is a straightforward
bug fix
### 4. SCOPE AND RISK ASSESSMENT
- **Lines changed**: ~10 lines, very small and localized
- **Files touched**: 1 file (net/wireless/util.c)
- **Subsystem**: WiFi cfg80211, mature and stable
- **Risk**: Very LOW - the exact same fix pattern was already applied to
the EHT version of this function and has been in stable since 6.1
- **Could break something**: Extremely unlikely - only changes
intermediate arithmetic precision, final result is more accurate
### 5. USER IMPACT
- **Who is affected**: Users with WiFi 6 (802.11ax) devices using high
bandwidth (160 MHz), high MCS (12/13), and multiple spatial streams
(NSS 3-8)
- **Visibility**: Users see incorrect bitrate reporting in tools like
`iw`, network managers, etc.
- **Severity**: Low-medium - cosmetic but confusing to users (reported
speed is much lower than actual)
- **Real-world scenario**: Modern high-end WiFi 6 devices can absolutely
hit these configurations
### 6. STABILITY INDICATORS
- **Testing**: Qualcomm developer tested on their hardware
- **Review**: Accepted by Johannes Berg (long-time wireless maintainer)
- **Precedent**: Same fix was applied to EHT function years ago (commit
18429c51c7ff6) and has proven stable
### 7. DEPENDENCY CHECK
- **Dependencies**: None - self-contained fix
- **Affected versions**: The HE MCS 12/13 support was added in v5.11, so
stable trees 5.11+ are affected
- **Backport complexity**: Trivial - the affected code exists unchanged
in all relevant stable trees
### Risk vs Benefit Trade-off
- **Benefit**: Corrects incorrect bitrate reporting for high-throughput
WiFi 6 configurations
- **Risk**: Minimal - follows proven fix pattern already in production
since kernel 6.1
- **Verdict**: HIGH benefit, VERY LOW risk
**YES**
net/wireless/util.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 4eb028ad16836..81d6d27d273cc 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -1561,12 +1561,14 @@ static u32 cfg80211_calculate_bitrate_he(struct rate_info *rate)
tmp = result;
tmp *= SCALE;
do_div(tmp, mcs_divisors[rate->mcs]);
- result = tmp;
/* and take NSS, DCM into account */
- result = (result * rate->nss) / 8;
+ tmp *= rate->nss;
+ do_div(tmp, 8);
if (rate->he_dcm)
- result /= 2;
+ do_div(tmp, 2);
+
+ result = tmp;
return result / 10000;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH AUTOSEL 6.18-5.10] wifi: mac80211: don't increment crypto_tx_tailroom_needed_cnt twice
[not found] <20260202214643.212290-1-sashal@kernel.org>
@ 2026-02-02 21:46 ` Sasha Levin
2026-02-02 21:46 ` [PATCH AUTOSEL 6.18-6.6] wifi: mac80211: correctly check if CSA is active Sasha Levin
2026-02-02 21:46 ` [PATCH AUTOSEL 6.18-5.10] wifi: cfg80211: Fix bitrate calculation overflow for HE rates Sasha Levin
2 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-02-02 21:46 UTC (permalink / raw)
To: patches, stable
Cc: Miri Korenblit, Johannes Berg, Sasha Levin, johannes,
linux-wireless
From: Miri Korenblit <miriam.rachel.korenblit@intel.com>
[ Upstream commit 3f3d8ff31496874a69b131866f62474eb24ed20a ]
In reconfig, in case the driver asks to disconnect during the reconfig,
all the keys of the interface are marked as tainted.
Then ieee80211_reenable_keys will loop over all the interface keys, and
for each one it will
a) increment crypto_tx_tailroom_needed_cnt
b) call ieee80211_key_enable_hw_accel, which in turn will detect that
this key is tainted, so it will mark it as "not in hardware", which is
paired with crypto_tx_tailroom_needed_cnt incrementation, so we get two
incrementations for each tainted key.
Then we get a warning in ieee80211_free_keys.
To fix it, don't increment the count in ieee80211_reenable_keys for
tainted keys
Reviewed-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Miri Korenblit <miriam.rachel.korenblit@intel.com>
Link: https://patch.msgid.link/20260118092821.4ca111fddcda.Id6e554f4b1c83760aa02d5a9e4e3080edb197aa2@changeid
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
So KEY_FLAG_TAINTED was introduced in v4.20. The bug being fixed is
likely in all kernels from 4.20 onward. Let me verify the current state
after the patch would be applied.
The patch being analyzed changes:
```c
list_for_each_entry(key, &sdata->key_list, list) {
increment_tailroom_need_count(sdata); // OLD: unconditional
ieee80211_key_enable_hw_accel(key);
}
```
to:
```c
list_for_each_entry(key, &sdata->key_list, list) {
if (!(key->flags & KEY_FLAG_TAINTED)) // NEW: skip for tainted
increment_tailroom_need_count(sdata);
ieee80211_key_enable_hw_accel(key);
}
```
This is correct because:
- For non-tainted keys: increment once here, and if HW accel succeeds,
it will be decremented in `ieee80211_key_enable_hw_accel()` (line
187). Net result: proper accounting.
- For tainted keys: skip here, let `ieee80211_key_enable_hw_accel()`
handle the increment (line 141) when clearing
`KEY_FLAG_UPLOADED_TO_HARDWARE`. Net result: proper accounting.
## Analysis Summary
**1. COMMIT MESSAGE ANALYSIS:**
- Clear explanation of the problem: double increment of
`crypto_tx_tailroom_needed_cnt` for tainted keys during reconfig
- The symptom is a warning in `ieee80211_free_keys`
- Reviewed-by from Johannes Berg (mac80211 maintainer) adds confidence
**2. CODE CHANGE ANALYSIS:**
- The bug: In `ieee80211_reenable_keys()`, for each key:
1. `increment_tailroom_need_count(sdata)` is called unconditionally
2. `ieee80211_key_enable_hw_accel(key)` is called
- For tainted keys (marked when driver requests disconnect during
reconfig), `ieee80211_key_enable_hw_accel()` will:
- Also call `increment_tailroom_need_count(sdata)` at line 141 if the
key was uploaded to hardware
- Clear the `KEY_FLAG_UPLOADED_TO_HARDWARE` flag
- Result: Two increments per tainted key instead of one
- The fix: Skip the first increment for tainted keys since they'll get
their increment inside `ieee80211_key_enable_hw_accel()`
**3. CLASSIFICATION:**
- This is a clear BUG FIX for a reference counting error
- Not a feature addition
- Fixes a real warning that users would see in kernel logs
**4. SCOPE AND RISK ASSESSMENT:**
- Very small change: adds a simple conditional check (1 line changed)
- Localized to mac80211 key handling
- Low risk: the logic is straightforward and has been reviewed by the
subsystem maintainer
- The worst-case if the fix is wrong would be the opposite accounting
error
**5. USER IMPACT:**
- WiFi users doing rekeys or going through suspend/resume/HW restart
with certain driver/firmware combinations could hit this
- The symptom is a kernel warning (WARN_ON_ONCE in
`ieee80211_free_keys`)
- It causes incorrect tailroom accounting which could potentially lead
to issues with software encryption
**6. STABILITY INDICATORS:**
- Reviewed-by: Johannes Berg (mac80211 maintainer)
- The code path and fix are straightforward
**7. DEPENDENCY CHECK:**
- KEY_FLAG_TAINTED exists since v4.20
- The function `ieee80211_reenable_keys()` exists in stable trees
- The code affected exists in all supported stable kernels
- No dependency on other commits - this is a standalone fix
The fix is small, surgical, reviewed by the subsystem maintainer, fixes
a real bug (warning + incorrect reference counting), and meets all
stable kernel criteria.
**YES**
net/mac80211/key.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index d5da7ccea66e0..04c8809173d7f 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -987,7 +987,8 @@ void ieee80211_reenable_keys(struct ieee80211_sub_if_data *sdata)
if (ieee80211_sdata_running(sdata)) {
list_for_each_entry(key, &sdata->key_list, list) {
- increment_tailroom_need_count(sdata);
+ if (!(key->flags & KEY_FLAG_TAINTED))
+ increment_tailroom_need_count(sdata);
ieee80211_key_enable_hw_accel(key);
}
}
--
2.51.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH AUTOSEL 6.18-6.6] wifi: mac80211: correctly check if CSA is active
[not found] <20260202214643.212290-1-sashal@kernel.org>
2026-02-02 21:46 ` [PATCH AUTOSEL 6.18-5.10] wifi: mac80211: don't increment crypto_tx_tailroom_needed_cnt twice Sasha Levin
@ 2026-02-02 21:46 ` Sasha Levin
2026-02-02 21:46 ` [PATCH AUTOSEL 6.18-5.10] wifi: cfg80211: Fix bitrate calculation overflow for HE rates Sasha Levin
2 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-02-02 21:46 UTC (permalink / raw)
To: patches, stable
Cc: Miri Korenblit, Johannes Berg, Sasha Levin, johannes,
linux-wireless
From: Miri Korenblit <miriam.rachel.korenblit@intel.com>
[ Upstream commit db1d0b6ab11f612ea8a327663a578c8946efeee9 ]
We are not adding an interface if an existing one is doing CSA.
But the check won't work for MLO station interfaces, since for those,
vif->bss_conf is zeroed out.
Fix this by checking if any link of the vif has an active CSA.
Reviewed-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Miri Korenblit <miriam.rachel.korenblit@intel.com>
Link: https://patch.msgid.link/20260111191912.7ceff62fc561.Ia38d27f42684d1cfd82d930d232bd5dea6ab9282@changeid
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a complete picture. Let me analyze this commit
comprehensively.
## Analysis
### 1. COMMIT MESSAGE ANALYSIS
**Subject**: "wifi: mac80211: correctly check if CSA is active"
The commit clearly describes a bug fix - the existing CSA (Channel
Switch Announcement) check doesn't work properly for MLO (Multi-Link
Operation) station interfaces. The message explains:
- The problem: "the check won't work for MLO station interfaces, since
for those, vif->bss_conf is zeroed out"
- The fix: "checking if any link of the vif has an active CSA"
**Tags present**:
- Reviewed-by: Johannes Berg (mac80211 maintainer) - strong credibility
signal
- Signed-off-by: Miri Korenblit (Intel wireless developer)
- Link to mailing list
No explicit Fixes: tag, but that's expected for commits we're reviewing.
### 2. CODE CHANGE ANALYSIS
**The Bug:**
The original code at line 379:
```c
if (nsdata->vif.bss_conf.csa_active)
return -EBUSY;
```
This check is used in `ieee80211_check_concurrent_iface()` to prevent
adding a new interface while an existing interface is performing a
channel switch operation.
**Why it's broken for MLO:**
For MLO (Multi-Link Operation) station interfaces introduced in WiFi 7
(802.11be):
- `vif->bss_conf` is zeroed out by design
- Each link has its own BSS configuration stored in `link_conf[link_id]`
- The CSA state (`csa_active`) is per-link, not per-VIF
- Checking `vif->bss_conf.csa_active` will always return `false` for MLO
interfaces, even if one of the links is actively switching channels
**The Fix:**
```c
struct ieee80211_link_data *link;
...
for_each_link_data(nsdata, link) {
if (link->conf->csa_active)
return -EBUSY;
}
```
This properly iterates through all valid links and checks if ANY link
has an active CSA. The `for_each_link_data` macro:
- For non-MLO interfaces: iterates once with the single (deflink) link
- For MLO interfaces: iterates through all links with bits set in
`valid_links`
**Technical mechanism**: The fix is correct because it unifies the check
- it works for both MLO and non-MLO interfaces while maintaining the
same semantics.
### 3. CLASSIFICATION
This is clearly a **bug fix**, not a new feature:
- The concurrent interface protection during CSA was always intended to
work
- It just doesn't work for the newer MLO interface type
- This is a correctness fix for existing functionality
### 4. SCOPE AND RISK ASSESSMENT
**Lines changed**: Small - adds ~6 lines, removes 2 lines (net +4)
**Files touched**: 1 file (net/mac80211/iface.c)
**Complexity**: Low - simple loop iteration replacing a single field
access
**Subsystem**: mac80211 WiFi subsystem - widely used but WiFi-specific
**Risk level**: LOW
- The change is straightforward and the logic is sound
- The `for_each_link_data` macro is designed for exactly this use case
- Reviewed by Johannes Berg, the mac80211 maintainer
- Failure mode: If the fix doesn't work, we're no worse than before
(still have the bug)
### 5. USER IMPACT
**Who is affected?**: Users with MLO-capable WiFi hardware (WiFi 7 /
802.11be devices)
**Severity**: Medium - Without this fix:
- A new interface could be incorrectly allowed to be added during an
ongoing channel switch on an MLO interface
- This could cause interference, connectivity issues, or undefined
behavior
- Channel switches are relatively rare events, so impact is somewhat
limited
### 6. STABILITY INDICATORS
- **Reviewed-by**: Johannes Berg (mac80211 maintainer) - strong positive
signal
- Author: Miri Korenblit from Intel - active wireless developer
### 7. DEPENDENCY CHECK
**Critical dependency**: This fix uses the `for_each_link_data` macro
which was introduced in commit f61c7b3d442be ("wifi: mac80211: Add link
iteration macro for link data") in kernel v6.16.
**Backport applicability**:
- **v6.16+**: Can be applied directly - has the macro
- **v6.1-v6.15**: Would require either:
1. Backporting the `for_each_link_data` macro first, OR
2. Writing an alternative fix using open-coded link iteration
MLO support was introduced in v6.1, so the bug exists in all kernels
v6.1 through current that have MLO station interface support.
### Summary
| Criteria | Assessment |
|----------|------------|
| Obviously correct | YES - simple iteration fix |
| Fixes real bug | YES - incorrect CSA check for MLO |
| Important issue | MEDIUM - affects interface addition during channel
switch |
| Small and contained | YES - 4 net lines changed |
| No new features | YES - fixes existing feature |
| Applies cleanly | DEPENDS - needs v6.16+ for macro dependency |
### VERDICT
This is a legitimate bug fix for MLO WiFi support:
- It fixes a real, demonstrable bug where CSA detection fails for MLO
interfaces
- The fix is small, surgical, and well-reviewed
- It doesn't introduce new features or change APIs
- It improves correctness of existing functionality
**However**, there are important caveats:
1. The fix has a dependency on the `for_each_link_data` macro
(introduced in v6.16)
2. For stable trees before v6.16, a backport would need the macro too or
use an alternative approach
3. MLO is relatively new technology (WiFi 7), so the affected user base
is smaller
Given that this fix:
- Is obviously correct and addresses a real bug
- Was reviewed by the subsystem maintainer
- Has low risk of regression
- Affects real user scenarios (channel switching during interface
operations)
The fix is appropriate for stable backporting to kernels that have both
MLO support and the `for_each_link_data` macro (v6.16+). For older
stable trees (v6.1-v6.15), a modified backport with the macro or open-
coded iteration would be needed.
**YES**
net/mac80211/iface.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 0ca55b9655a7f..72c129478da08 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -350,6 +350,8 @@ static int ieee80211_check_concurrent_iface(struct ieee80211_sub_if_data *sdata,
/* we hold the RTNL here so can safely walk the list */
list_for_each_entry(nsdata, &local->interfaces, list) {
if (nsdata != sdata && ieee80211_sdata_running(nsdata)) {
+ struct ieee80211_link_data *link;
+
/*
* Only OCB and monitor mode may coexist
*/
@@ -376,8 +378,10 @@ static int ieee80211_check_concurrent_iface(struct ieee80211_sub_if_data *sdata,
* will not add another interface while any channel
* switch is active.
*/
- if (nsdata->vif.bss_conf.csa_active)
- return -EBUSY;
+ for_each_link_data(nsdata, link) {
+ if (link->conf->csa_active)
+ return -EBUSY;
+ }
/*
* The remaining checks are only performed for interfaces
--
2.51.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH AUTOSEL 6.18-5.10] wifi: cfg80211: Fix bitrate calculation overflow for HE rates
[not found] <20260202214643.212290-1-sashal@kernel.org>
2026-02-02 21:46 ` [PATCH AUTOSEL 6.18-5.10] wifi: mac80211: don't increment crypto_tx_tailroom_needed_cnt twice Sasha Levin
2026-02-02 21:46 ` [PATCH AUTOSEL 6.18-6.6] wifi: mac80211: correctly check if CSA is active Sasha Levin
@ 2026-02-02 21:46 ` Sasha Levin
2 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-02-02 21:46 UTC (permalink / raw)
To: patches, stable
Cc: Veerendranath Jakkam, Johannes Berg, Sasha Levin, johannes,
linux-wireless
From: Veerendranath Jakkam <veerendranath.jakkam@oss.qualcomm.com>
[ Upstream commit a3034bf0746d88a00cceda9541534a5721445a24 ]
An integer overflow occurs in cfg80211_calculate_bitrate_he() when
calculating bitrates for high throughput HE configurations.
For example, with 160 MHz bandwidth, HE-MCS 13, HE-NSS 4, and HE-GI 0,
the multiplication (result * rate->nss) overflows the 32-bit 'result'
variable before division by 8, leading to significantly underestimated
bitrate values.
The overflow occurs because the NSS multiplication operates on a 32-bit
integer that cannot accommodate intermediate values exceeding
4,294,967,295. When overflow happens, the value wraps around, producing
incorrect bitrates for high MCS and NSS combinations.
Fix this by utilizing the 64-bit 'tmp' variable for the NSS
multiplication and subsequent divisions via do_div(). This approach
preserves full precision throughout the entire calculation, with the
final value assigned to 'result' only after completing all operations.
Signed-off-by: Veerendranath Jakkam <veerendranath.jakkam@oss.qualcomm.com>
Link: https://patch.msgid.link/20260109-he_bitrate_overflow-v1-1-95575e466b6e@oss.qualcomm.com
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
I can see the buggy code is still present in this version (the fix
hasn't been applied yet to this version of util.c). This is the code
being patched. Let me double check the diff shows exactly what I expect:
Looking at the diff again:
```c
- result = tmp;
/* and take NSS, DCM into account */
- result = (result * rate->nss) / 8;
+ tmp *= rate->nss;
+ do_div(tmp, 8);
if (rate->he_dcm)
- result /= 2;
+ do_div(tmp, 2);
+
+ result = tmp;
```
The fix:
1. Removes `result = tmp` after the MCS division (keeps value in 64-bit
`tmp`)
2. Multiplies NSS in 64-bit: `tmp *= rate->nss` instead of 32-bit
`result * rate->nss`
3. Uses `do_div(tmp, 8)` instead of 32-bit division
4. Uses `do_div(tmp, 2)` for DCM instead of 32-bit division
5. Only assigns to 32-bit `result` at the end when the value is
guaranteed to fit
This matches exactly the same fix pattern that was applied to
`cfg80211_calculate_bitrate_eht()` in commit 18429c51c7ff6.
## Summary Analysis
### 1. COMMIT MESSAGE ANALYSIS
- **Subject**: Clearly indicates a bug fix for bitrate calculation
overflow in HE rates
- **Body**: Provides detailed technical explanation of the overflow
scenario (160 MHz, MCS 13, NSS 4, GI 0)
- **Root cause**: 32-bit integer overflow when multiplying `result *
rate->nss` before division by 8
- **Fix approach**: Use 64-bit `tmp` variable throughout the
calculation, assign to 32-bit `result` only at the end
- **Tags**: Has Signed-off-by from both Qualcomm developer and Johannes
Berg (wireless maintainer)
### 2. CODE CHANGE ANALYSIS
- **Bug mechanism**: When `result` (a u32) is large (e.g., 0x44b85dd4 =
1,152,933,332), multiplying by NSS 4 produces 0x112e17750
(4,611,733,328) which overflows u32 max (0xFFFFFFFF = 4,294,967,295)
- **Result of bug**: The overflow wraps to 0x12e17750, producing a
bitrate of ~3,959 (400 Mbps) instead of correct ~57,646 (5.76 Gbps)
- **Fix correctness**: The fix keeps all arithmetic in 64-bit until the
final assignment, preventing overflow
- **DCM handling**: Also moved to 64-bit for consistency and safety
### 3. CLASSIFICATION
- **Type**: Bug fix (integer overflow causing incorrect bitrate
reporting)
- **Security**: Not a security issue, but affects user-visible data
correctness
- **Exception categories**: Not applicable - this is a straightforward
bug fix
### 4. SCOPE AND RISK ASSESSMENT
- **Lines changed**: ~10 lines, very small and localized
- **Files touched**: 1 file (net/wireless/util.c)
- **Subsystem**: WiFi cfg80211, mature and stable
- **Risk**: Very LOW - the exact same fix pattern was already applied to
the EHT version of this function and has been in stable since 6.1
- **Could break something**: Extremely unlikely - only changes
intermediate arithmetic precision, final result is more accurate
### 5. USER IMPACT
- **Who is affected**: Users with WiFi 6 (802.11ax) devices using high
bandwidth (160 MHz), high MCS (12/13), and multiple spatial streams
(NSS 3-8)
- **Visibility**: Users see incorrect bitrate reporting in tools like
`iw`, network managers, etc.
- **Severity**: Low-medium - cosmetic but confusing to users (reported
speed is much lower than actual)
- **Real-world scenario**: Modern high-end WiFi 6 devices can absolutely
hit these configurations
### 6. STABILITY INDICATORS
- **Testing**: Qualcomm developer tested on their hardware
- **Review**: Accepted by Johannes Berg (long-time wireless maintainer)
- **Precedent**: Same fix was applied to EHT function years ago (commit
18429c51c7ff6) and has proven stable
### 7. DEPENDENCY CHECK
- **Dependencies**: None - self-contained fix
- **Affected versions**: The HE MCS 12/13 support was added in v5.11, so
stable trees 5.11+ are affected
- **Backport complexity**: Trivial - the affected code exists unchanged
in all relevant stable trees
### Risk vs Benefit Trade-off
- **Benefit**: Corrects incorrect bitrate reporting for high-throughput
WiFi 6 configurations
- **Risk**: Minimal - follows proven fix pattern already in production
since kernel 6.1
- **Verdict**: HIGH benefit, VERY LOW risk
**YES**
net/wireless/util.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 4eb028ad16836..81d6d27d273cc 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -1561,12 +1561,14 @@ static u32 cfg80211_calculate_bitrate_he(struct rate_info *rate)
tmp = result;
tmp *= SCALE;
do_div(tmp, mcs_divisors[rate->mcs]);
- result = tmp;
/* and take NSS, DCM into account */
- result = (result * rate->nss) / 8;
+ tmp *= rate->nss;
+ do_div(tmp, 8);
if (rate->he_dcm)
- result /= 2;
+ do_div(tmp, 2);
+
+ result = tmp;
return result / 10000;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-02-02 21:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260202214643.212290-1-sashal@kernel.org>
2026-02-02 21:46 ` [PATCH AUTOSEL 6.18-5.10] wifi: mac80211: don't increment crypto_tx_tailroom_needed_cnt twice Sasha Levin
2026-02-02 21:46 ` [PATCH AUTOSEL 6.18-6.6] wifi: mac80211: correctly check if CSA is active Sasha Levin
2026-02-02 21:46 ` [PATCH AUTOSEL 6.18-5.10] wifi: cfg80211: Fix bitrate calculation overflow for HE rates Sasha Levin
[not found] <20260128223332.2806589-1-sashal@kernel.org>
2026-01-28 22:33 ` Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox