Re: [PATCH B 01/10] OMAP2/3 clock: combine clkdm, clkdm_name intounion in struct clk

[Paul Walmsley]

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/