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

[Xuanqiang Luo]

在 2026/8/28 06:13, Kaiwen Shi 写道:

@@ -599,25 +603,38 @@ int mac802154_perform_association(struct 
ieee802154_sub_if_data *sdata,
                goto clear_assoc;
        }

- if (local->assoc_status != IEEE802154_ASSOCIATION_SUCCESSFUL) {

-               if (local->assoc_status == IEEE802154_PAN_AT_CAPACITY)
+       /* The association is complete: clear the associating bit and snapshot
+        * the result under the same lock, so a second (e.g. malicious) ASSOC
+        * RESP can no longer pass the recheck below and overwrite
+        * assoc_status/assoc_addr before they are consumed.
+        */
+       spin_lock(&local->assoc_lock);
+       clear_bit(IEEE802154_IS_ASSOCIATING, &local->ongoing);

I think this clear_bit() should be moved to
mac802154_process_association_resp(), after saving the first valid
response and before calling complete().

Clearing it only after wait_for_completion() returns may still leave a
window, since the woken waiter may not acquire assoc_lock before the
next work item runs. Clearing it earlier in the response handler ensures
that subsequent responses fail the in-lock
IEEE802154_IS_ASSOCIATING check and cannot overwrite the saved result.

+       resp_status = local->assoc_status;
+       resp_short_addr = local->assoc_addr;
+       spin_unlock(&local->assoc_lock);
+
+       if (resp_status != IEEE802154_ASSOCIATION_SUCCESSFUL) {
+               if (resp_status == IEEE802154_PAN_AT_CAPACITY)
                        ret = -ERANGE;
                else
                        ret = -EPERM;

dev_warn(&sdata->dev->dev,

                         "Negative ASSOC RESP received from %8phC: %s\n", 
&ceaddr,
-                        local->assoc_status == IEEE802154_PAN_AT_CAPACITY ?
+                        resp_status == IEEE802154_PAN_AT_CAPACITY ?
                         "PAN at capacity" : "access denied");
-               goto clear_assoc;
+               return ret;
        }

- ret = 0;

-       *short_addr = local->assoc_addr;
+       *short_addr = resp_short_addr;
+
+       return 0;

clear_assoc:

+       spin_lock(&local->assoc_lock);
        clear_bit(IEEE802154_IS_ASSOCIATING, &local->ongoing);
-       local->assoc_dev = NULL;
+       spin_unlock(&local->assoc_lock);

return ret;

  }
@@ -639,19 +656,23 @@ int mac802154_process_association_resp(struct 
ieee802154_sub_if_data *sdata,
                     dest->mode != IEEE802154_EXTENDED_ADDRESSING))
                return -EINVAL;

- if (unlikely(dest->extended_addr != wpan_dev->extended_addr ||

-                    src->extended_addr != local->assoc_dev->extended_addr))
+       spin_lock(&local->assoc_lock);
+       if (unlikely(!test_bit(IEEE802154_IS_ASSOCIATING, &local->ongoing) ||
+                    dest->extended_addr != wpan_dev->extended_addr ||
+                    src->extended_addr != local->assoc_dev_extended_addr)) {
+               spin_unlock(&local->assoc_lock);
                return -ENODEV;
+       }

memcpy(&resp_pl, skb->data, sizeof(resp_pl));

        local->assoc_addr = resp_pl.short_addr;
        local->assoc_status = resp_pl.status;

here

+       complete(&local->assoc_done);
+       spin_unlock(&local->assoc_lock);

dev_dbg(&skb->dev->dev,

                "ASSOC RESP 0x%x received from %8phC, getting short address 
%04x\n",
-               local->assoc_status, &deaddr, local->assoc_addr);
-
-       complete(&local->assoc_done);
+               resp_pl.status, &deaddr, resp_pl.short_addr);

return 0;

  }

base-commit: 8d3ae59288f1e7d58d76558a6ee96d533bc5019f

Thanks,
Xuanqiang