Re: [PATCH net v4] mac802154: fix data race and NULL deref on local->assoc_dev

[Miquel Raynal <miquel.raynal@bootlin.com>]

On 30/08/2026 at 07:05:51 +08, Kaiwen Shi <skwkevin@mail.ustc.edu.cn> wrote:

> local->assoc_dev is shared between the association path and the
> association-response worker without common synchronization.
>
> mac802154_perform_association() stores the coordinator pointer and waits
> for a response. Its timeout and error paths clear the pointer and return
> to mac802154_associate(), which may then free the coordinator object.
> Meanwhile, mac802154_rx_mac_cmd_worker() may observe the associating bit
> and enter mac802154_process_association_resp(), which dereferences
> assoc_dev.
>
> The worker's bit test and the handler's pointer dereference are not
> atomic with respect to cleanup. Cleanup can clear assoc_dev between them,
> causing a NULL dereference, or free the coordinator while the response
> handler still uses the pointer.
>
> The recorded result is exposed to the same window. assoc_status and
> assoc_addr are written by the handler but read by the association path
> while the associating bit is still set, so a second response for the same
> request - a malicious one, for instance - can replace them between those
> reads and leave the caller with an incoherent status and address pair.
>
> The response handler only needs the coordinator extended address.
> Replace assoc_dev with a cached address, removing the pointer lifetime
> dependency. Protect the cached address and the associating bit with a
> dedicated spinlock. A READ_ONCE()/WRITE_ONCE() pair would not guarantee
> an atomic __le64 access on all 32-bit architectures.
>
> wpan_dev->association_lock cannot be reused here: nl802154_associate()
> holds it across rdev_associate(), hence for the whole of
> mac802154_perform_association() including the wait for the response.
> A response handler taking that lock would only get it once the
> association has already given up.
>
> Reset the completion, publish the cached address, and set the associating
> bit while holding the lock. The response handler takes the lock, rechecks
> the bit and the cached address, records the response, clears the bit, and
> only then completes the waiter. Thus cleanup cannot pass the handler
> between its state check and completion, and the cached 64-bit value
> cannot tear.
>
> The handler clears the bit before completing, not the woken waiter:
> otherwise complete() is issued under the lock and a second (e.g.
> malicious) response can reacquire it before the waiter and replace the
> result. So a wait that returns success implies the bit is already clear,
> and the success and negative-response paths return directly. The
> transmit-error and timeout paths still clear it under assoc_lock, which
> serializes any racing response against the cleanup while the call returns
> the error it already selected. Both paths snapshot assoc_status and
> assoc_addr under the same lock.
>
> Both users run in process context, so a plain spinlock is sufficient.
> The lock is not held while waiting for the completion.
>
> Suggested-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Suggested-by: Xuanqiang Luo <xuanqiang.luo@linux.dev>
> Fixes: fefd19807fe9 ("mac802154: Handle associating")
> Cc: stable@vger.kernel.org
> Signed-off-by: Kaiwen Shi <skwkevin@mail.ustc.edu.cn>
> ---

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl