From: Sidhanta Sahu sidhanta.sahu@oss.qualcomm.com
[ Upstream commit c426497fa2055c8005196922e7d29c41d7e0948a ]
rx_stats for each arsta is allocated when adding a station. arsta->rx_stats will be freed when a station is removed.
Redundant allocations are occurring when the same station is added multiple times. This causes ath12k_mac_station_add() to be called multiple times, and rx_stats is allocated each time. As a result there is memory leaks.
Prevent multiple allocations of rx_stats when ath12k_mac_station_add() is called repeatedly by checking if rx_stats is already allocated before allocating again. Allocate arsta->rx_stats if arsta->rx_stats is NULL respectively.
Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1 Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
Signed-off-by: Sidhanta Sahu sidhanta.sahu@oss.qualcomm.com Signed-off-by: Muna Sinada muna.sinada@oss.qualcomm.com Reviewed-by: Mahendran P quic_mahep@quicinc.com Link: https://patch.msgid.link/20250326213538.2214194-1-muna.sinada@oss.qualcomm.c... 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. Here's my extensive analysis: ## Analysis of the Commit ### 1. Nature of the Bug Fix This commit fixes a **memory leak** in the ath12k WiFi driver. The specific issue is that `rx_stats` memory is allocated every time `ath12k_mac_station_add()` is called, but when the same station is added multiple times, it creates redundant allocations without freeing the previous allocation, leading to memory leaks. ### 2. Code Changes Analysis The fix is **minimal and surgical**: **Before the fix:** ```c arsta->rx_stats = kzalloc(sizeof(*arsta->rx_stats), GFP_KERNEL); if (!arsta->rx_stats) { ret = -ENOMEM; goto dec_num_station; } ``` **After the fix:** ```c if (!arsta->rx_stats) { arsta->rx_stats = kzalloc(sizeof(*arsta->rx_stats), GFP_KERNEL); if (!arsta->rx_stats) { ret = -ENOMEM; goto dec_num_station; } } ``` The change simply adds a NULL check before allocation, ensuring memory is only allocated if `arsta->rx_stats` is NULL. ### 3. Risk Assessment - **Low Risk**: The change is a simple conditional check that prevents redundant allocations - **No Architectural Changes**: The fix doesn't alter any fundamental driver behavior or data structures - **Contained**: The change is localized to a single function and doesn't affect other subsystems - **No New Features**: This is purely a bugfix with no new functionality ### 4. Comparison with Similar Commits Looking at the provided similar commits, this follows the exact same pattern as **approved backport candidates**: - **Similar Commit #2 (YES)**: "Fix memory leak in rx_desc and tx_desc" - Similar memory leak fix - **Similar Commit #3 (YES)**: "fix tx_stats memory leak" - Identical pattern fixing stats memory leak in ath10k - **Similar Commit #4 (YES)**: "fix memory leak in WMI firmware stats" - Memory leak fix in ath11k - **Similar Commit #5 (YES)**: "fix memory leak in ath12k_dp_rx_peer_frag_setup()" - Memory leak fix in ath12k All these approved commits share the same characteristics: - Fix memory leaks in wireless drivers - Make minimal, surgical changes - Have low regression risk - Address real user- affecting bugs ### 5. User Impact Memory leaks can cause: - System memory exhaustion over time - Degraded system performance - Potential system instability in resource-constrained environments - Affects users with ath12k hardware (QCN9274, WCN7850 as mentioned in testing) ### 6. Testing Evidence The commit includes comprehensive testing on real hardware: - QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1 - WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3 ### 7. Stable Tree Compliance This commit perfectly fits stable tree criteria: - ✅ Fixes an important bug (memory leak) - ✅ No new features introduced - ✅ No architectural changes - ✅ Minimal risk of regression - ✅ Confined to a specific driver subsystem - ✅ Clear, well-documented fix ### 8. Code Context Verification From examining the kernel tree, I confirmed: - The function `ath12k_mac_station_add()` can indeed be called multiple times (from state transitions and link assignment) - Proper error handling exists with `kfree(arsta->rx_stats)` in the cleanup paths - The fix addresses a real scenario where station addition can happen multiple times This is a textbook example of a commit that should be backported: it fixes a real memory leak bug with a minimal, low-risk change that follows established patterns for successful backports in the wireless subsystem.
drivers/net/wireless/ath/ath12k/mac.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c index 17415f7a1f51d..bcb5b049bba18 100644 --- a/drivers/net/wireless/ath/ath12k/mac.c +++ b/drivers/net/wireless/ath/ath12k/mac.c @@ -5562,10 +5562,13 @@ static int ath12k_mac_station_add(struct ath12k *ar, ar->max_num_stations); goto exit; } - arsta->rx_stats = kzalloc(sizeof(*arsta->rx_stats), GFP_KERNEL); + if (!arsta->rx_stats) { - ret = -ENOMEM; - goto dec_num_station; + arsta->rx_stats = kzalloc(sizeof(*arsta->rx_stats), GFP_KERNEL); + if (!arsta->rx_stats) { + ret = -ENOMEM; + goto dec_num_station; + } }
peer_param.vdev_id = arvif->vdev_id;