bes2600: Patch F — backport cw1200 mainline bug fixes (3 commits) #4

Merged
marfrit merged 3 commits from bes2600/cw1200-fix-backports into cleanups 2026-05-07 19:30:13 +00:00
Collaborator

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 of atomic_add(1, &x)atomic_inc(&x) for readability. Pure cosmetic, zero functional risk.

F2 — 65a4c39 bes2600: fix missing destroy_workqueue() on error in init_common

Backport of 7ec8a926188e (Qinglang Miao / Hulk Robot, 2020-11-19, Fixes: a910e4a94f69). Two error paths in bes2600_init_common (queue_stats_init failure, queue_init failure) were leaking the workqueue. Add destroy_workqueue(hw_priv->workqueue) before ieee80211_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_scan

Backport of 86760e0dfe36 / 4f68ef64cd7f (Jia-Ju Bai, 2018-12-14). The probe-request SKB allocated by ieee80211_probereq_get() BEFORE scan.lock + conf_lock are taken can be freed concurrently by bss_info_changed. Reorder to acquire both locks BEFORE the SKB allocation. Plus reorder cleanup paths so dev_kfree_skb() runs BEFORE up() to close the touched-after-unlock window. Two affected functions (bes2600_hw_scan and bes2600_hw_sched_scan_start); the latter is #ifdef ROAM_OFFLOAD so compiled-out at default build but fixed for consistency.

Skipped: F3 — d98c24617a83 wifi: cw1200: Fix locking in error paths

Cw1200 commit added mutex_unlock(&priv->conf_mutex) to two error paths in cw1200_wow_suspend because cw1200_wow_resume (which the error paths call) only released scan.lock, not conf_mutex. bes2600's bes2600_wow_resume already releases both scan.lock and conf_lock (pm.c:510, 536). Audited the early-error paths at pm.c:358-363 and 366-369 — they call wow_resume which cleans up correctly. No bug to fix here. Audited and recorded as task #32.

Build verified

bes2600.ko builds clean on ohm sandbox with all 3 commits applied — srcversion A9438692D6A8698F92AEEA1, 0 warnings, 0 errors.

Risk

  • F1: zero (mechanical, no semantic change)
  • F2: very low (defensive cleanup; only runs on probe failure path)
  • F4: low (lock ordering / cleanup ordering — verified against cw1200's reference fix; same code shape; same fix shape)

Test plan

  • User reboots ohm to load Patch-F module (ohm reboot is permitted while wired path + dev allocation hold per feedback_user_pushes_reboot_button override)
  • Verify load + association: lsmod, dmesg, iw dev wlan0 link
  • Smoke-test scan: iw dev wlan0 scan (exercises F4 path)
  • No need for full Phase 7 stress ramp — these are non-hot-path fixes
  • If anything regresses: /var/tmp/bes2600.patchB.rollback.ko is the fallback

Independence 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:

  • spelling/typo fixes (irrelevant)
  • debugfs return-value cleanup (bes2600 didn't inherit that pattern)
  • maybe-uninitialized warning (compiler-version-specific)
  • signedness bug in load_firmware (bes2600 has its own bes_fw.c with different code, not applicable)

If future cw1200 fixes land, mining ~/src/linux-rockchip is the right starting point (80K-commit tree, ~5min query budget per fix).

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 of `atomic_add(1, &x)` → `atomic_inc(&x)` for readability. Pure cosmetic, zero functional risk. ### F2 — `65a4c39 bes2600: fix missing destroy_workqueue() on error in init_common` Backport of `7ec8a926188e` (Qinglang Miao / Hulk Robot, 2020-11-19, Fixes: a910e4a94f69). Two error paths in `bes2600_init_common` (queue_stats_init failure, queue_init failure) were leaking the workqueue. Add `destroy_workqueue(hw_priv->workqueue)` before `ieee80211_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_scan` Backport of `86760e0dfe36` / `4f68ef64cd7f` (Jia-Ju Bai, 2018-12-14). The probe-request SKB allocated by `ieee80211_probereq_get()` BEFORE `scan.lock + conf_lock` are taken can be freed concurrently by `bss_info_changed`. Reorder to acquire both locks BEFORE the SKB allocation. Plus reorder cleanup paths so `dev_kfree_skb()` runs BEFORE `up()` to close the touched-after-unlock window. Two affected functions (`bes2600_hw_scan` and `bes2600_hw_sched_scan_start`); the latter is `#ifdef ROAM_OFFLOAD` so compiled-out at default build but fixed for consistency. ## Skipped: F3 — `d98c24617a83 wifi: cw1200: Fix locking in error paths` Cw1200 commit added `mutex_unlock(&priv->conf_mutex)` to two error paths in `cw1200_wow_suspend` because `cw1200_wow_resume` (which the error paths call) only released `scan.lock`, not `conf_mutex`. **bes2600's `bes2600_wow_resume` already releases both `scan.lock` and `conf_lock`** (pm.c:510, 536). Audited the early-error paths at pm.c:358-363 and 366-369 — they call `wow_resume` which cleans up correctly. No bug to fix here. Audited and recorded as task #32. ## Build verified `bes2600.ko` builds clean on ohm sandbox with all 3 commits applied — srcversion `A9438692D6A8698F92AEEA1`, 0 warnings, 0 errors. ## Risk - F1: zero (mechanical, no semantic change) - F2: very low (defensive cleanup; only runs on probe failure path) - F4: low (lock ordering / cleanup ordering — verified against cw1200's reference fix; same code shape; same fix shape) ## Test plan - [ ] User reboots ohm to load Patch-F module (ohm reboot is permitted while wired path + dev allocation hold per feedback_user_pushes_reboot_button override) - [ ] Verify load + association: `lsmod`, `dmesg`, `iw dev wlan0 link` - [ ] Smoke-test scan: `iw dev wlan0 scan` (exercises F4 path) - [ ] No need for full Phase 7 stress ramp — these are non-hot-path fixes - [ ] If anything regresses: `/var/tmp/bes2600.patchB.rollback.ko` is the fallback ## Independence 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: - spelling/typo fixes (irrelevant) - debugfs return-value cleanup (bes2600 didn't inherit that pattern) - maybe-uninitialized warning (compiler-version-specific) - signedness bug in load_firmware (bes2600 has its own bes_fw.c with different code, not applicable) If future cw1200 fixes land, mining `~/src/linux-rockchip` is the right starting point (80K-commit tree, ~5min query budget per fix).
claude-noether added 3 commits 2026-05-07 19:28:16 +00:00
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.com
Two 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
marfrit merged commit 6a6aa243a4 into cleanups 2026-05-07 19:30:13 +00:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marfrit/bes2600-dkms#4