bes2600: Patch F — backport cw1200 mainline bug fixes (3 commits) #4
Reference in New Issue
Block a user
Delete Branch "bes2600/cw1200-fix-backports"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Backport series from upstream cw1200 (drivers/net/wireless/st/cw1200/) bug-fix history. The cw1200 driver is bes2600's structural ancestor; many bugs found and fixed upstream over 16 years still apply to bes2600 because we forked off and accreted features rather than tracking fixes.
Mined
~/src/linux-rockchip(mmind/linux-rockchip mirror, 80,268 commits, 228 cw1200-touching).Commits
F1 —
4bc0a34 bes2600: replace a set of atomic_add()Backport of
07f995ca1951(Yejune Deng, 2020-11-10). Mechanical: 7 sites ofatomic_add(1, &x)→atomic_inc(&x)for readability. Pure cosmetic, zero functional risk.F2 —
65a4c39 bes2600: fix missing destroy_workqueue() on error in init_commonBackport of
7ec8a926188e(Qinglang Miao / Hulk Robot, 2020-11-19, Fixes: a910e4a94f69). Two error paths inbes2600_init_common(queue_stats_init failure, queue_init failure) were leaking the workqueue. Adddestroy_workqueue(hw_priv->workqueue)beforeieee80211_free_hw(hw)in both. Cw1200's identical bug was found by Hulk Robot static analysis.F4 —
b717251 bes2600: fix concurrency UAF in bes2600_hw_scan and sched_scanBackport of
86760e0dfe36/4f68ef64cd7f(Jia-Ju Bai, 2018-12-14). The probe-request SKB allocated byieee80211_probereq_get()BEFOREscan.lock + conf_lockare taken can be freed concurrently bybss_info_changed. Reorder to acquire both locks BEFORE the SKB allocation. Plus reorder cleanup paths sodev_kfree_skb()runs BEFOREup()to close the touched-after-unlock window. Two affected functions (bes2600_hw_scanandbes2600_hw_sched_scan_start); the latter is#ifdef ROAM_OFFLOADso compiled-out at default build but fixed for consistency.Skipped: F3 —
d98c24617a83 wifi: cw1200: Fix locking in error pathsCw1200 commit added
mutex_unlock(&priv->conf_mutex)to two error paths incw1200_wow_suspendbecausecw1200_wow_resume(which the error paths call) only releasedscan.lock, notconf_mutex. bes2600'sbes2600_wow_resumealready releases bothscan.lockandconf_lock(pm.c:510, 536). Audited the early-error paths at pm.c:358-363 and 366-369 — they callwow_resumewhich cleans up correctly. No bug to fix here. Audited and recorded as task #32.Build verified
bes2600.kobuilds clean on ohm sandbox with all 3 commits applied — srcversionA9438692D6A8698F92AEEA1, 0 warnings, 0 errors.Risk
Test plan
lsmod,dmesg,iw dev wlan0 linkiw dev wlan0 scan(exercises F4 path)/var/tmp/bes2600.patchB.rollback.kois the fallbackIndependence from Patch C v2 / Patch C v3
Patch F is independent of the structural Bug #5 work. C-prep / C-v2 / C-v3 (per PR #10 / task #35) build on the cleanups branch separately. No conflict expected — F touches main.c, scan.c, bh.c (mostly non-overlapping with the sdio_rx_work hot path).
Lessons not backported (for completeness)
Reviewed the full 17 cw1200-fix list; selected 4 high-value candidates. Other cw1200 fixes considered:
If future cw1200 fixes land, mining
~/src/linux-rockchipis the right starting point (80K-commit tree, ~5min query budget per fix).Backport of cw1200 mainline commit 07f995ca1951 ("cw1200: replace a set of atomic_add()", 2020-11-10). atomic_inc() reads more naturally than atomic_add(1, &x). Mechanical change, no functional impact. 7 sites: 6 in bh.c (bh_term, bh_rx x2, bh_tx x3) and 1 in itp.c (awaiting_confirm). Two of the bh_rx and three of the bh_tx sites are inside the cw1200-ancestor #if 0 block; replaced anyway to keep the file consistent with cw1200 mainline source style. Cherry-picked from upstream Linux: 07f995ca1951 cw1200: replace a set of atomic_add() Author: Yejune Deng <yejune.deng@gmail.com> Signed-off-by: Kalle Valo <kvalo@codeaurora.org> Link: https://lore.kernel.org/r/1604991491-27908-1-git-send-email-yejune.deng@gmail.comTwo error paths between create_singlethread_workqueue() (~main.c:489) and the success-path destroy_workqueue() in unregister_common (~609) return without cleaning up the workqueue, leaking it on probe failure: 1. bes2600_queue_stats_init() failure 2. bes2600_queue_init() failure (any of the 4 TID queues) Both call ieee80211_free_hw(hw); return NULL — without first destroy_workqueue(hw_priv->workqueue). Add it. Backport of cw1200 mainline commit 7ec8a926188e ("cw1200: fix missing destroy_workqueue() on error in cw1200_init_common", 2020-11-19), which fixed the identical bug in the same code shape we inherited. Reported on cw1200 by Hulk Robot. Cherry-picked from upstream Linux: 7ec8a926188e cw1200: fix missing destroy_workqueue() on error Author: Qinglang Miao <miaoqinglang@huawei.com> Reported-by: Hulk Robot <hulkci@huawei.com> Signed-off-by: Kalle Valo <kvalo@codeaurora.org> Link: https://lore.kernel.org/r/20201119070842.1011-1-miaoqinglang@huawei.com Fixes: a910e4a94f69 ("cw1200: add driver for the ST-E CW1100 & CW1200 WLAN chipsets")bes2600_bss_info_changed() and bes2600_hw_scan() can run concurrently. The probe-request SKB allocated by ieee80211_probereq_get() before scan.lock + conf_lock are taken can be touched by a concurrent bss_info_changed (via wsm_set_template_frame's path) while we hold no lock. Reorder to acquire both locks BEFORE the SKB allocation. Also reorder cleanup paths so dev_kfree_skb() runs BEFORE up() — otherwise a small window exists where the SKB has been touched but the lock has been released, allowing concurrent code to also touch it. Three sites fixed: - bes2600_hw_scan: lock-take + ENOMEM cleanup + wsm_set_template_frame error cleanup + success-path SKB free + lock release order - bes2600_sched_scan_start (#ifdef ROAM_OFFLOAD): same three sub-fixes (compiled-out at default build, fixed for consistency) - All success/error paths: dev_kfree_skb before up() Backport of cw1200 mainline commit 86760e0dfe36 ("cw1200: Fix concurrency use-after-free bugs in cw1200_hw_scan()", 2018-12-14), which fixed the identical bug in the same code shape we inherited. That commit was merged from upstream 4f68ef64cd7f. Cherry-picked from upstream Linux: 86760e0dfe36 cw1200: Fix concurrency use-after-free bugs in cw1200_hw_scan() Author: Jia-Ju Bai <baijiaju1990@gmail.com> Link: https://lore.kernel.org/r/20181214035521.7575-1-baijiaju1990@gmail.com