linux-kernel - Re: [PATCH 1/2] Input: spear-keyboard - Fix a double put in spear_kbd

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 for Android: free password hash cracker in your pocket

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

Message-ID: <ZqK7HQMtV8oavTsM@google.com>
Date: Thu, 25 Jul 2024 13:52:45 -0700
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc: vipulkumar.samar@...com, viresh.kumar@...aro.org,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
	kernel-janitors@...r.kernel.org
Subject: Re: [PATCH 1/2] Input: spear-keyboard - Fix a double put in
 spear_kbd_remove()

Hi Christophe,

On Thu, Jul 25, 2024 at 10:46:49PM +0200, Christophe JAILLET wrote:
> The 'input_dev' is a managed resource allocated with
> devm_input_allocate_device(), so there is no need to call
> input_unregister_device() in the remove function.
> 
> In fact, this call was correctly removed in commit 6102752eb354 ("Input:
> spear-keyboard - switch to using managed resources"), but silently
> re-introduced later in the commit in Fixes.

This change is incorrect as it leads to an active and enabled clock
being unprepared to early. We need to unregister input device which in
turn will call spear_kbd_close() if needed which will disable the clock
in question. Only after that we can unprepare it.

There is also no double put as input core will recognize that input
device was unregistered explicitly and will not attempt to unregister it
2nd time through devm:

void input_unregister_device(struct input_dev *dev)
{
	if (dev->devres_managed) {
		WARN_ON(devres_destroy(dev->dev.parent,
					devm_input_device_unregister,
					devm_input_device_match,
					dev));
		__input_unregister_device(dev);
		/*
		 * We do not do input_put_device() here because it will be done
		 * when 2nd devres fires up.
		 */
	} else {
		__input_unregister_device(dev);
		input_put_device(dev);
	}
}

> 
> Fixes: 9336648978c2 ("Input: spear-keyboard - add clk_{un}prepare() support")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@...adoo.fr>
> ---
> Compile tested-only
> ---
>  drivers/input/keyboard/spear-keyboard.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/input/keyboard/spear-keyboard.c b/drivers/input/keyboard/spear-keyboard.c
> index 557d00a667ce..5d9fc8dc9433 100644
> --- a/drivers/input/keyboard/spear-keyboard.c
> +++ b/drivers/input/keyboard/spear-keyboard.c
> @@ -276,7 +276,6 @@ static void spear_kbd_remove(struct platform_device *pdev)
>  {
>  	struct spear_kbd *kbd = platform_get_drvdata(pdev);
>  
> -	input_unregister_device(kbd->input);
>  	clk_unprepare(kbd->clk);
>  }
>  
> -- 
> 2.45.2
> 

Thanks.

-- 
Dmitry