Hi Xuanqiang, > 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.
Good catch, you are right. complete() is issued while the handler still holds assoc_lock, so between the wake-up and the waiter taking that lock another response can get in, pass the recheck and replace assoc_status/assoc_addr before perform_association() has consumed them. I reproduced it on a debug kernel: on v3 the bit is still set when the wait returns, later responses keep being accepted after that point, and the association ends up with an address from one of them instead of the first one. With the clear_bit() moved, none is accepted. So v4 does what you suggested: clear_bit() right after the result is stored and before complete(), both under assoc_lock. The timeout and error paths still clear it under the lock. The success and negative paths do not need to any more, because a wait that returns success now means the handler has already cleared the bit. One thing I got wrong earlier, while I am at it. In my v2 reply I said wpan_dev->association_lock is not held on either of the two paths involved. That was wrong. nl802154_associate() takes it around rdev_associate(), so it is held for the whole of mac802154_perform_association(), the wait for the response included. That rules out reusing it here: the handler would only get that mutex once the association has already timed out. The v4 commit message states this correctly. A v4 follows. Thanks, Kaiwen
