Re: [PATCH] iwlegacy/4965: Cancel deferred work on device init failure

[Stanislaw Gruszka]

Hi

On Tue, Mar 04, 2025 at 11:16:03AM +0800, Wentao Liang wrote:
> In __il4965_up(), deferred work is not canceled in time when device
> initialization fails. This is harmless if the device has not started.
> However, in il4965_bg_restart(), if the device remains operational
> in any state other than S_FW_ERROR or S_EXIT_PENDING, a dereference
> operation needs to be performed when __il4965_up() fails.
>
> Add il4965_cancel_deferred_work() to the failure path of
> __il4965_up() to prevent potential errors. Even if the current code
> does not exhibit the described issues, adding this change can prevent
> future problems at minimal cost, improving the robustness of the code.

I think preventing potential future problems in this legacy driver
is not necessary since the driver will not be developed further.

> Signed-off-by: Wentao Liang <vulab@xxxxxxxxxxx>
> ---
>  drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/wireless/intel/iwlegacy/4965-mac.c b/drivers/net/wireless/intel/iwlegacy/4965-mac.c
> index 05c4af41bdb9..3b21bd79f3a9 100644
> --- a/drivers/net/wireless/intel/iwlegacy/4965-mac.c
> +++ b/drivers/net/wireless/intel/iwlegacy/4965-mac.c
> @@ -5591,6 +5591,8 @@ __il4965_up(struct il_priv *il)
>  	__il4965_down(il);
>  	clear_bit(S_EXIT_PENDING, &il->status);
>  
> +	il4965_cancel_deferred_work(il);

We can not cancel here as we have il->mutex locked, it will
deadlock in case of some pending work that holds the mutex.

>  	/* tried to restart and config the device for as long as our
>  	 * patience could withstand */
>  	IL_ERR("Unable to initialize device after %d attempts.\n", i);
> -- 
> 2.42.0.windows.2
>