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

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

[Wentao Liang]

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.

Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 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);
+
 	/* 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

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@iscas.ac.cn>
> ---
>  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
>