linux-kernel - Re: [Cocci] [PATCH v13 3/6] clk: Make clk API return per-user struct clk instances

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]

Date:	Wed, 04 Feb 2015 15:26:19 -0800
From:	Stephen Boyd <sboyd@...eaurora.org>
To:	Quentin Lambert <lambert.quentin@...il.com>,
	Mike Turquette <mturquette@...aro.org>,
	Julia Lawall <julia.lawall@...6.fr>
CC:	Paul Walmsley <paul@...an.com>,
	Tomeu Vizoso <tomeu.vizoso@...labora.com>,
	Tony Lindgren <tony@...mide.com>, linux-kernel@...r.kernel.org,
	t-kristo@...com, linux-omap@...r.kernel.org, cocci@...teme.lip6.fr,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [Cocci] [PATCH v13 3/6] clk: Make clk API return per-user struct
 clk instances

On 02/03/15 08:04, Quentin Lambert wrote:
> Hello,
> Julia asked me to have a look and see if I can help.
>
> I have found these three cases using Coccinnelle in the mach-omap2
> directory.
>
>
>
> ./arch/arm/mach-omap2/clkt_clksel.c
> @@ -67,7 +67,6 @@ static const struct clksel *_get_clksel_
>                 return NULL;
>
>         for (clks = clk->clksel; clks->parent; clks++)
>                 if (clks->parent == src_clk)

This probably needs to compare hw pointers again given that it's in the
clk-provider code. It would be nice if we could use indices instead though.

>                         break; /* Found the requested parent */
>
>         if (!clks->parent) {
> ./arch/arm/mach-omap2/dpll3xxx.c
> @@ -551,7 +549,6 @@ int omap3_noncore_dpll_set_rate(struct c
>         if (!dd)
>                 return -EINVAL;
>
>         if (__clk_get_parent(hw->clk) != dd->clk_ref)

This one is what this thread has started on about. Comparing hw pointers
is ok for now...

>                 return -EINVAL;
>
>         if (dd->last_rounded_rate == 0)
> ./arch/arm/mach-omap2/timer.c
> @@ -298,7 +298,6 @@ static int __init omap_dm_timer_init_one
>         if (IS_ERR(src))
>                 return PTR_ERR(src);
>
>         if (clk_get_parent(timer->fclk) != src) {

This one looks like an optimization. We can just always try to set the
parent and if it's already the current parent then the core should bail
out early without an error. So the fix would be to just remove the if
condition.

>                 r = clk_set_parent(timer->fclk, src);
>                 if (r < 0) {
>                         pr_warn("%s: %s cannot set source\n", __func__,
>
>
> Are they instances of your issue?

Yes these all look like instances of the problem.

> If you want me to I can enlarge the search to other directories.

Yes please do. And if you could share the coccinelle patch that would be
great. Thanks.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/