gpio: omap: handle clk_prepare failure in probe

[PATCH] gpio: omap: handle clk_prepare failure in probe

Posted by Jiawen Liu 1 month, 1 week ago

omap_gpio_probe() ignores the return value of clk_prepare(bank->dbck).
If clk_prepare fails, the clock is not prepared, but bank->dbck_flag
remains true. Later, omap_gpio_remove() or the probe error path calls
clk_unprepare(bank->dbck) unconditionally when dbck_flag is true,
leading to an unbalanced clock operation.

Check the return value of clk_prepare in omap_gpio_probe. On failure,
clear dbck_flag and return the error, preventing unbalanced
clk_unprepare in remove or error paths.

Signed-off-by: jiawen <1298662399@qq.com>
---
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1462,7 +1462,12 @@
 				"Could not get gpio dbck. Disable debounce\n");
 			bank->dbck_flag = false;
 		} else {
-			clk_prepare(bank->dbck);
+			ret = clk_prepare(bank->dbck);
+			if (ret) {
+				dev_err(dev, "Could not prepare gpio dbck\n");
+				bank->dbck_flag = false;
+				return ret;
+			}
 		}
 	}

Re: [PATCH] gpio: omap: handle clk_prepare failure in probe

Posted by Andreas Kemnade 1 month, 1 week ago

On Tue, 18 Aug 2026 17:08:54 +0400
Jiawen Liu <1298662399@qq.com> wrote:

> omap_gpio_probe() ignores the return value of clk_prepare(bank->dbck).
> If clk_prepare fails, the clock is not prepared, but bank->dbck_flag
> remains true. Later, omap_gpio_remove() or the probe error path calls
> clk_unprepare(bank->dbck) unconditionally when dbck_flag is true,
> leading to an unbalanced clock operation.
> 
> Check the return value of clk_prepare in omap_gpio_probe. On failure,
> clear dbck_flag and return the error, preventing unbalanced
> clk_unprepare in remove or error paths.
> 
> Signed-off-by: jiawen <1298662399@qq.com>
> ---
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -1462,7 +1462,12 @@
>  				"Could not get gpio dbck. Disable debounce\n");
>  			bank->dbck_flag = false;
>  		} else {
> -			clk_prepare(bank->dbck);
> +			ret = clk_prepare(bank->dbck);
> +			if (ret) {
> +				dev_err(dev, "Could not prepare gpio dbck\n");
> +				bank->dbck_flag = false;
> +				return ret;
> +			}
>
What about simply using devm_clk_get_prepared() here? That would simplify
things a lot, given that AFAIK,  prepare is a no-op here anyways.

Regards,
Andreas

Re: [PATCH] gpio: omap: handle clk_prepare failure in probe

Posted by Markus Elfring 1 month, 1 week ago

…
> Check the return value of clk_prepare in omap_gpio_probe. On failure,
> clear dbck_flag and return the error, preventing unbalanced
> clk_unprepare in remove or error paths.

How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?

See also once more:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v7.2#n34
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2#n145
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2#n792

Regards,
Markus

Re: [PATCH] gpio: omap: handle clk_prepare failure in probe

Posted by Andreas Kemnade 1 month, 1 week ago

On Thu, 20 Aug 2026 11:19:00 +0200
Markus Elfring <Markus.Elfring@web.de> wrote:

> …
> > Check the return value of clk_prepare in omap_gpio_probe. On failure,
> > clear dbck_flag and return the error, preventing unbalanced
> > clk_unprepare in remove or error paths.  
> 
> How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?
> 
About adding this to stable:
does it bother anyone?
Is this actually being used with clocks having prepare() ops?
I do not see anything there. SoC-internal clocks are typically not
so easy being rewired.
To be clear, this should be fixed in -next to be prepared if something
more fundamental in clock handling changes.

Regards,
Andreas

> See also once more:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v7.2#n34
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2#n145
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2#n792
> 
> Regards,
> Markus
> 
>

Re: gpio: omap: handle clk_prepare failure in probe

Posted by Markus Elfring 1 month, 1 week ago

> About adding this to stable:
> does it bother anyone?

I imagine that it can be safer to avoid return value ignorance a bit more.
https://cwe.mitre.org/data/definitions/252.html

Regards,
Markus

Re: gpio: omap: handle clk_prepare failure in probe

Posted by Andreas Kemnade 1 month, 1 week ago

On Thu, 20 Aug 2026 12:47:05 +0200
Markus Elfring <Markus.Elfring@web.de> wrote:

> > About adding this to stable:
> > does it bother anyone?  
> 
> I imagine that it can be safer to avoid return value ignorance a bit more.
> https://cwe.mitre.org/data/definitions/252.html
> 
Well, if that would depend on user input, esp. over the network, it would be
clear.
But here you need to patch the devicetree to add a clock there
that has a prepare() in the ancestry. shows the desired behavior to exploit
something.

And then the error check is done by clk_enable() anyways.
BTW: here the return value of that is not checked. That is the more
interesting issue here.

Quoting stable kernel rules:
"
 - No "This could be a problem..." type of things like a "theoretical race
    condition", unless an explanation of how the bug can be exploited is also
    provided.
"
From taking that verbatim, I would say no CC stable.
But pragmatically, also to avoid noise in any security scanner, I would agree
to a CC stable here.

Regards,
Andreas