| Message ID | E1Ye59E-0001BB-7z@rmk-PC.arm.linux.org.uk (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
On 04/03/15 10:12, Russell King wrote:
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Some commit text would be nice for us 5 years from now when we wonder
why this patch was applied.
I suspect the commit text would go like this:
clk_add_alias() calls clk_get() followed by clk_put() but in between
those two calls it saves away the struct clk pointer to a clk_lookup
structure. This leaves the 'clk' member of the clk_lookup pointing at
freed memory on configurations where CONFIG_COMMON_CLK=y. This is a
problem because clk_get_sys() will eventually try to dereference the
freed pointer by calling __clk_get_hw() on it. Fix this by saving away
the struct clk_hw pointer instead of the struct clk pointer so that when
we try to create a per-user struct clk in clk_get_sys() we don't
dereference a junk pointer.
Now the question is does any of this matter for the 4.0 release. From
what I can tell, the answer is no.
$ git grep 'clk_add_alias' v4.0-rc6
v4.0-rc6:arch/arm/mach-davinci/da850.c: clk_add_alias("async", da850_cpufreq_device.name,
v4.0-rc6:arch/arm/mach-omap1/board-nokia770.c: clk_add_alias("hwa_sys_ck", NULL, "bclk", NULL);
v4.0-rc6:arch/arm/mach-pxa/eseries.c: clk_add_alias("CLK_CK48M", e740_t7l66xb_device.name,
v4.0-rc6:arch/arm/mach-pxa/eseries.c: clk_add_alias("CLK_CK3P6MI", e750_tc6393xb_device.name,
v4.0-rc6:arch/arm/mach-pxa/eseries.c: clk_add_alias("CLK_CK3P6MI", e800_tc6393xb_device.name,
v4.0-rc6:arch/arm/mach-pxa/lubbock.c: clk_add_alias("SA1111_CLK", NULL, "GPIO11_CLK", NULL);
v4.0-rc6:arch/arm/mach-pxa/tosa.c: clk_add_alias("CLK_CK3P6MI", tc6393xb_device.name, "GPIO11_CLK", NULL);
v4.0-rc6:arch/mips/alchemy/common/clock.c: clk_add_alias(t->alias, NULL, t->base, NULL);
v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("wdt", NULL, "ahb", NULL);
v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("uart", NULL, "ahb", NULL);
v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("wdt", NULL, "ahb", NULL);
v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("uart", NULL, "ahb", NULL);
v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("wdt", NULL, "ahb", NULL);
v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("uart", NULL, "ahb", NULL);
v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("wdt", NULL, "ahb", NULL);
v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("uart", NULL, "ref", NULL);
v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("wdt", NULL, "ref", NULL);
v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("uart", NULL, "ref", NULL);
v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("wdt", NULL, "ref", NULL);
v4.0-rc6:arch/mips/ath79/clock.c: clk_add_alias("uart", NULL, "ref", NULL);
v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c: clk_add_alias("fck", "sh-tmu-sh3.0", "peripheral_clk", NULL);
v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c: clk_add_alias("fck", "sh-tmu.0", "peripheral_clk", NULL);
v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c: clk_add_alias("fck", "sh-tmu.1", "peripheral_clk", NULL);
v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c: clk_add_alias("fck", "sh-tmu.2", "peripheral_clk", NULL);
v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c: clk_add_alias("fck", "sh-mtu2", "peripheral_clk", NULL);
v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c: clk_add_alias("fck", "sh-cmt-16.0", "peripheral_clk", NULL);
v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c: clk_add_alias("fck", "sh-cmt-32.0", "peripheral_clk", NULL);
v4.0-rc6:arch/sh/kernel/cpu/clock-cpg.c: clk_add_alias("sci_ick", NULL, "peripheral_clk", NULL);
All of these architectures and platforms have CONFIG_COMMON_CLK=n, so
there doesn't seem to be any regression that these patches are fixing.
That isn't to say the patches are bad, just that they aren't urgent for
the upcoming release.
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c index 5d7746d19445..8e676eafc823 100644 --- a/drivers/clk/clkdev.c +++ b/drivers/clk/clkdev.c @@ -205,7 +205,7 @@ struct clk *clk_get_sys(const char *dev_id, const char *con_id) if (!cl) goto out; - clk = __clk_create_clk(__clk_get_hw(cl->clk), dev_id, con_id); + clk = __clk_create_clk(cl->clk_hw, dev_id, con_id); if (IS_ERR(clk)) goto out; @@ -243,18 +243,26 @@ void clk_put(struct clk *clk) } EXPORT_SYMBOL(clk_put); -void clkdev_add(struct clk_lookup *cl) +static void __clkdev_add(struct clk_lookup *cl) { mutex_lock(&clocks_mutex); list_add_tail(&cl->node, &clocks); mutex_unlock(&clocks_mutex); } + +void clkdev_add(struct clk_lookup *cl) +{ + if (!cl->clk_hw) + cl->clk_hw = __clk_get_hw(cl->clk); + __clkdev_add(cl); +} EXPORT_SYMBOL(clkdev_add); void clkdev_add_table(struct clk_lookup *cl, size_t num) { mutex_lock(&clocks_mutex); while (num--) { + cl->clk_hw = __clk_get_hw(cl->clk); list_add_tail(&cl->node, &clocks); cl++; } @@ -271,7 +279,7 @@ struct clk_lookup_alloc { }; static struct clk_lookup * __init_refok -vclkdev_alloc(struct clk *clk, const char *con_id, const char *dev_fmt, +vclkdev_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt, va_list ap) { struct clk_lookup_alloc *cla; @@ -280,7 +288,7 @@ vclkdev_alloc(struct clk *clk, const char *con_id, const char *dev_fmt, if (!cla) return NULL; - cla->cl.clk = clk; + cla->cl.clk_hw = hw; if (con_id) { strlcpy(cla->con_id, con_id, sizeof(cla->con_id)); cla->cl.con_id = cla->con_id; @@ -301,7 +309,7 @@ clkdev_alloc(struct clk *clk, const char *con_id, const char *dev_fmt, ...) va_list ap; va_start(ap, dev_fmt); - cl = vclkdev_alloc(clk, con_id, dev_fmt, ap); + cl = vclkdev_alloc(__clk_get_hw(clk), con_id, dev_fmt, ap); va_end(ap); return cl; @@ -362,7 +370,7 @@ int clk_register_clkdev(struct clk *clk, const char *con_id, return PTR_ERR(clk); va_start(ap, dev_fmt); - cl = vclkdev_alloc(clk, con_id, dev_fmt, ap); + cl = vclkdev_alloc(__clk_get_hw(clk), con_id, dev_fmt, ap); va_end(ap); if (!cl) @@ -393,8 +401,8 @@ int clk_register_clkdevs(struct clk *clk, struct clk_lookup *cl, size_t num) return PTR_ERR(clk); for (i = 0; i < num; i++, cl++) { - cl->clk = clk; - clkdev_add(cl); + cl->clk_hw = __clk_get_hw(clk); + __clkdev_add(cl); } return 0; diff --git a/include/linux/clkdev.h b/include/linux/clkdev.h index eb9b38a79b43..cd93b215e3af 100644 --- a/include/linux/clkdev.h +++ b/include/linux/clkdev.h @@ -22,6 +22,7 @@ struct clk_lookup { const char *dev_id; const char *con_id; struct clk *clk; + struct clk_hw *clk_hw; }; #define CLKDEV_INIT(d, n, c) \
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> --- drivers/clk/clkdev.c | 24 ++++++++++++++++-------- include/linux/clkdev.h | 1 + 2 files changed, 17 insertions(+), 8 deletions(-)