Re: [PATCH] wifi: iwlegacy: serialize watchdog updates with device teardown
From: Stanislaw Gruszka
Date: Tue Oct 06 2026 - 04:40:40 EST
On Sun, Oct 04, 2026 at 07:45:19PM +0800, Runyu Xiao wrote:
> The writable wd_timeout debugfs file changes il->cfg->wd_timeout, but the
> iwl3945 configuration is shared and const. The handler also rearms the
The cfg in il_priv is not const. What would possibly make sense is
take _all_ modified fields out, and make it const then.
> watchdog without taking il->mutex. The down paths hold this mutex, delete
> the timer, and then free the TX queues, so an unlocked debugfs write can
> rearm the timer after deletion. The callback can then access the queues
> after they have been freed.
>
> Store wd_timeout in per-device state. Serialize the debugfs update with
> the down paths and only arm the watchdog while TX queues exist. Use
> READ_ONCE() and WRITE_ONCE() for accesses that do not hold il->mutex.
>
> Fixes: 1dc80798a8ca ("iwlegacy: constify local structures")
This is wrong tag. At least for watchdog stop races.
> --- a/drivers/net/wireless/intel/iwlegacy/debug.c
> +++ b/drivers/net/wireless/intel/iwlegacy/debug.c
> @@ -1284,8 +1284,12 @@ il_dbgfs_wd_timeout_write(struct file *file, const char __user *user_buf,
> if (timeout < 0 || timeout > IL_MAX_WD_TIMEOUT)
> timeout = IL_DEF_WD_TIMEOUT;
>
> - il->cfg->wd_timeout = timeout;
> - il_setup_watchdog(il);
> + mutex_lock(&il->mutex);
> + WRITE_ONCE(il->wd_timeout, timeout);
> + if (il->txq)
> + il_setup_watchdog(il);
This will not work for 4965. Possibly il->is_open could be used as
check. But maybe we should just remove possibility of setting
wd_timeout, I need to think about it.
Regards
Stanislaw
> + mutex_unlock(&il->mutex);
> +
> return count;
> }
>
> --
> 2.34.1