Re: [PATCH net] wifi: ath9k: Fix potential spin_lock() before spin_lock_init()

[Toke Høiland-Jørgensen]

Thomas Fourier <fourier.thomas@xxxxxxxxx> writes:

> The function ath9k_init_wmi() initializes wmi->wmi_lock. It is called in
> ath9k_htc_probe_device(), and the priv->initialized flag is set.
> However, the ath9k_wmi_event_tasklet takes the lock before checking the
> priv->initialized flag, so the lock may not be initialized before
> being taken.  This could be the case, for example, if the spin_lock_init()
> is reordered with tasklet_setup() in ath9k_init_wmi() by the compiler or
> CPU.
>
> There is a write memory barrier before setting the priv->initialized,
> but no corresponding read memory barrier is used after checking the
> flag.
>
> Move priv->initialized at the start of ath9k_wmi_event_tasklet() and
> add a corresponding read memory barrier.
>
> Fixes: 24355fcb0d4c ("wifi: ath9k: delay all of ath9k_wmi_event_tasklet() until init is complete")
> Signed-off-by: Thomas Fourier <fourier.thomas@xxxxxxxxx>
> ---
>  drivers/net/wireless/ath/ath9k/wmi.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/wmi.c b/drivers/net/wireless/ath/ath9k/wmi.c
> index 284e8c13b043..df4a3a625536 100644
> --- a/drivers/net/wireless/ath/ath9k/wmi.c
> +++ b/drivers/net/wireless/ath/ath9k/wmi.c
> @@ -146,6 +146,15 @@ void ath9k_wmi_event_tasklet(struct tasklet_struct *t)
>  	unsigned long flags;
>  	u16 cmd_id;
>  
> +	/* Check if ath9k_htc_probe_device() completed. */
> +	if (!data_race(priv->initialized))
> +		return;

Moving this out of the loop changes behaviour: Before, the loop would
keep spinning waiting for initialisation, now we just exit. I don't see
any guarantee that we'll come back here, so this has the risk of
stalling things. We'll need to re-schedule the tasklet before returning
if we're moving the check here.

-Toke