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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250304164407.GB67578@wp.pl>
Date: Tue, 4 Mar 2025 17:44:07 +0100
From: Stanislaw Gruszka <stf_xl@...pl>
To: Wentao Liang <vulab@...as.ac.cn>
Cc: kvalo@...nel.org, linux-wireless@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iwlegacy/4965: Cancel deferred work on device init
 failure

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@...as.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
>