Re: [PATCH] ftrace: unsigned idx cannot be less than 0

From: Steven Rostedt
Date: Tue Jan 06 2009 - 10:50:01 EST



[ added Liming to CC ]

On Fri, 2 Jan 2009, Frederic Weisbecker wrote:

> On Fri, Jan 02, 2009 at 03:49:43PM +0100, Roel Kluin wrote:
> > // vi kernel/trace/ftrace.c +787
> > struct ftrace_iterator {
> > ...
> > unsigned idx;
> > ...
> > };
> >
> > idx is unsigned and cannot be less than 0.
> >
> > Signed-off-by: Roel Kluin <roel.kluin@xxxxxxxxx>
> > ---
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index 2f32969..a344add 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
> > @@ -842,7 +842,7 @@ static void *t_start(struct seq_file *m, loff_t *pos)
> > void *p = NULL;
> >
> > if (*pos > 0) {
> > - if (iter->idx < 0)
> > + if (iter->idx == 0)
> > return p;
> > (*pos)--;
> > iter->idx--;
>
>
> Hi Roel,
>
> I'm not sure this is the right fix.
> If you look at t_next, if there is no more page to look at,
> iter_idx takes -1.
>
> A 0 value would mean: we are in the first index on the page, which means
> there is something to read and we don't want to return NULL.
>
> I guess that would be better to turn idx into a signed int.

Correct. This bug was added by:

50cdaf08a8ec1d7f43987705da7aff7cf949708f
ftrace: improve seq_operation of ftrace

So the correct fix is to turn it into a signed int.

Thanks,

-- Steve
--
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/