On Sat, 31 Jan 2009, Russell King - ARM Linux wrote:
> On Tue, Jan 27, 2009 at 07:44:08PM -0700, Paul Walmsley wrote:
> > diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c
> > index 55c5d67..7aa09f5 100644
> > --- a/arch/arm/mach-omap2/clock.c
> > +++ b/arch/arm/mach-omap2/clock.c
> > @@ -77,17 +77,17 @@ void omap2_init_clk_clkdm(struct clk *clk)
> > {
> > struct clockdomain *clkdm;
> >
> > - if (!clk->clkdm_name)
> > + if (!clk->clkdm.name)
> > return;
> >
> > - clkdm = clkdm_lookup(clk->clkdm_name);
> > + clkdm = clkdm_lookup(clk->clkdm.name);
> > if (clkdm) {
> > pr_debug("clock: associated clk %s to clkdm %s\n",
> > - clk->name, clk->clkdm_name);
> > - clk->clkdm = clkdm;
> > + clk->name, clk->clkdm.name);
> > + clk->clkdm.ptr = clkdm;
> > } else {
> > pr_debug("clock: could not associate clk %s to "
> > - "clkdm %s\n", clk->name, clk->clkdm_name);
> > + "clkdm %s\n", clk->name, clk->clkdm.name);
> > }
>
> This is unsafe - if the clock domain can not be found, you leave the
> union pointing at the string, and there's no way for this to prevent
> the clock from being registered.
>
> The result is that:
>
> > - if (clk->clkdm)
> > - omap2_clkdm_clk_disable(clk->clkdm, clk);
> > + if (clk->clkdm.ptr)
> > + omap2_clkdm_clk_disable(clk->clkdm.ptr, clk);
>
> and similar places will pass the pointer to the string, potentially
> causing an oops, or worse, data corruption due to scribbing over
> someone elses memory.
Agreed. Now that omap2_init_clk_clkdm() is called by the
OMAP arch-specific clk_register(), this should be pretty easy to
implement. Will send a patch that applies on top of F 06.
- Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
