[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kernel-janitors
Subject:    Re: [patch] sh: clkfwk: potential null deref in debug code
From:       Guennadi Liakhovetski <g.liakhovetski () gmx ! de>
Date:       2011-03-23 14:01:00
Message-ID: Pine.LNX.4.64.1103231459290.6836 () axis700 ! grange
[Download RAW message or body]

On Wed, 23 Mar 2011, Paul Mundt wrote:

> On Sun, Mar 20, 2011 at 04:57:50PM +0100, Guennadi Liakhovetski wrote:
> > Hi Dan
> > 
> > On Sun, 20 Mar 2011, Dan Carpenter wrote:
> > 
> > > This function is designed to accept a NULL for "best_freq" but the
> > > debug code dereferences it unconditionally.
> > > 
> > > Signed-off-by: Dan Carpenter <error27@gmail.com>
> > > ---
> > > Btw.  Smatch complains that "best" could NULL as well, but I don't
> > > know if actually that's possible so I left it as is.
> > > 
> > > diff --git a/drivers/sh/clk/core.c b/drivers/sh/clk/core.c
> > > index 5f63c3b..dee971c 100644
> > > --- a/drivers/sh/clk/core.c
> > > +++ b/drivers/sh/clk/core.c
> > > @@ -616,7 +616,7 @@ long clk_round_parent(struct clk *clk, unsigned long target,
> > >  
> > >  		pr_debug("%u / %lu = %lu, / %lu = %lu, best %lu, parent %u\n",
> > >  			 freq->frequency, div, freq_high, div + 1, freq_low,
> > > -			 *best_freq, best->frequency);
> > > +			 best_freq ? *best_freq : -1, best->frequency);
> > 
> > What about another occasion of dereferencing best_freq at the top of the 
> > clk_round_parent() function:
> > 
> > 	if (!parent) {
> > 		*parent_freq = 0;
> > 		*best_freq = clk_round_rate(clk, target);
> > 		return abs(target - *best_freq);
> > 	}
> > 
> > ?
> > 
> Do we actually have users that are passing in NULL for best_freq? I don't
> recall writing this code, so I assume you did :-)

That seems very likely;) Yes, that's from my patch, and so far we have 
only one user - ap4evb_clk_optimize() and it doesn't pass NULL to the fn. 
But perhaps it is better to be consistent and if the variable is checked 
for NULL on some occasions, it should be checked everywhere.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]