linux-kernel - Re: [Patch v6] i2c: pnx: Fix potential deadlock warning from del_timer

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <dudh4jdce3yxwv5yw345gw23diwparhwvsl4jrpsyzpv3sgge3@ojtdgsdgwcor>
Date: Tue, 2 Jul 2024 01:01:08 +0200
From: Andi Shyti <andi.shyti@...nel.org>
To: Piotr Wojtaszczyk <piotr.wojtaszczyk@...esys.com>
Cc: Vladimir Zapolskiy <vz@...ia.com>, 
	linux-arm-kernel@...ts.infradead.org, linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [Patch v6] i2c: pnx: Fix potential deadlock warning from
 del_timer_sync() call in isr

Hi Piotr,

On Fri, Jun 28, 2024 at 05:25:42PM GMT, Piotr Wojtaszczyk wrote:
> When del_timer_sync() is called in an interrupt context it throws a warning
> because of potential deadlock. The timer is used only to exit from
> wait_for_completion() after a timeout so replacing the call with
> wait_for_completion_timeout() allows to remove the problematic timer and
> its related functions altogether.

nice patch! I still would like an ack from Vladimir.

> Signed-off-by: Piotr Wojtaszczyk <piotr.wojtaszczyk@...esys.com>

Fixes: 41561f28e76a ("i2c: New Philips PNX bus driver")
Cc: Vitaly Wool <vitaly.wool@...sulko.com>
Cc: <stable@...r.kernel.org> # v2.6.20+

...

> @@ -653,7 +624,10 @@ static int i2c_pnx_probe(struct platform_device *pdev)
>  	alg_data->adapter.algo_data = alg_data;
>  	alg_data->adapter.nr = pdev->id;
>  
> -	alg_data->timeout = I2C_PNX_TIMEOUT_DEFAULT;
> +	alg_data->timeout = msecs_to_jiffies(I2C_PNX_TIMEOUT_DEFAULT);
> +	if (alg_data->timeout <= 1)
> +		alg_data->timeout = 2;

I don't see the need for this check. The default timeout is
defined as 10.

Thanks,
Andi