Re: [PATCH net] ath9k_htc: fix possibly missing barrier in ath9k_htc_rxep()

[Toke Høiland-Jørgensen]

Jeff Johnson <jeff.johnson@xxxxxxxxxxxxxxxx> writes:

> On 8/3/2026 3:44 AM, Thomas Fourier wrote:
>> ath9k_rx_init() initialises the rx buffer and its lock then calls a
>> memory barrier and then sets the priv->rx.initialized flag. However,
>> ath9k_htc_rxep() reads that flag and imidiatly takes the lock. This may
>
> s/imidiatly/immediately/
>
>> cause the lock to be taken while not fully initialized.
>> 
>> Add a barrier to prevent speculative read of the lock before checking
>> the initized flag.
>
> s/initized/initialized/ (or initialised)
>
>> 
>> Fixes: b0ec7e55fce6 ("ath9k_htc: fix NULL pointer dereference at ath9k_htc_rxep()")
>> Cc: <stable@xxxxxxxxxxxxxxx>
>> Signed-off-by: Thomas Fourier <fourier.thomas@xxxxxxxxx>
>> ---
>>  drivers/net/wireless/ath/ath9k/htc_drv_txrx.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
>> index bed7ea2425a0..97d61f3f0aad 100644
>> --- a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
>> +++ b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
>> @@ -1145,6 +1145,12 @@ void ath9k_htc_rxep(void *drv_priv, struct sk_buff *skb,
>>  	if (!data_race(priv->rx.initialized))
>>  		goto err;
>>  
>> +	/*
>> +	 * Make sure all the ath9k_rx_init() memory writes are visible before
>> +	 * proceeding.
>> +	 */
>> +	smp_rmb();
>> +
>>  	spin_lock_irqsave(&priv->rx.rxbuflock, flags);
>>  	list_for_each_entry(tmp_buf, &priv->rx.rxbuf, list) {
>>  		if (!tmp_buf->in_process) {
>
> I decided to run this though my review agent, which has the following
> analysis:

[...]

> So my question: Are there any flaws with that analysis?

Well, my conclusion was basically that adding the smp_rmb() is not
actively wrong, even if it's not exactly idiomatic.

But sure, we can go with smp_store_release/load_acquire instead; might
as well fix up that other tasklet issue while I'm at it...

-Toke