Hi Kaiwen, > - cache the coordinator extended address instead of retaining the > assoc_dev pointer, as suggested by Miquel; > - use a plain spinlock to serialize the cached address and associating > bit, including 64-bit address accesses on 32-bit architectures;
Can you justify the choice of a spinlock vs. mutex here? This is an open question, not a request for changes. > - reset the completion under the same lock, recheck the associating bit > in the response handler, and complete before releasing the lock; > - use the response payload in the debug message; > - add Cc: [email protected]. > ... > --- a/net/mac802154/scan.c > +++ b/net/mac802154/scan.c > @@ -578,9 +578,11 @@ int mac802154_perform_association(struct > ieee802154_sub_if_data *sdata, > return ret; > } > > - local->assoc_dev = coord; > + spin_lock(&local->assoc_lock); > reinit_completion(&local->assoc_done); > + local->assoc_dev_extended_addr = coord->extended_addr; > set_bit(IEEE802154_IS_ASSOCIATING, &local->ongoing); > + spin_unlock(&local->assoc_lock); > > ret = ieee802154_mlme_tx_one_locked(local, sdata, skb); > if (ret) { Shouldn't we also make sure that accessing assoc_status/assoc_addr is serialized? Typically, I believe the IS_ASSOCIATING bit should be cleared earlier in mac802154_perform_association(), just after the wait_for_completion call returns. This way, in case we get two responses for the same request (maybe a malicious one), it will prevent the possibility to get incoherent assoc_status and assoc_address. Thanks, Miquèl
