[Crash-utility] [PATCH] Fix ps -l buffer overflow problem

Dave Anderson anderson at redhat.com
Wed Jan 14 19:54:50 UTC 2009


----- "Jeff Moyer" <jmoyer at redhat.com> wrote:

> Bernhard Walle <bwalle at suse.de> writes:
> 
> > That should also go mainline.
> >
> >
> > Signed-off-by : Sachin Sant <sachinp at in.ibm.com>
> > Acked-by: Bernhard Walle <bwalle at suse.de>
> >
> >
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > task.c |    2 +-
> >
> >
> > That should also go mainline.
> >
> >
> > Signed-off-by : Sachin Sant <sachinp at in.ibm.com>
> > Acked-by: Bernhard Walle <bwalle at suse.de>
> >
> > diff --git a/task.c b/task.c
> > --- a/task.c
> > +++ b/task.c
> > @@ -2902,7 +2902,7 @@
> >  {
> >  	int i, c;
> >  	struct task_context *tcp;
> > -	char format[10];
> > +	char format[15];
> 
> 
>         c = strlen(buf);
>         sprintf(format, "[%c%dll%c]  ", '%', c,
>                 pc->output_radix == 10 ? 'u' : 'x');
> 
> Looks like it should be 11, no?  You have 6 characters + '\0' + the %d,
> which can be up to 1500 (BUFSIZE).  Of course, it wouldn't kill us to
> use snprintf, either.

You're right, it can only be a maximum of 11 bytes, i.e., either "[%20llu]  "
(decimal) or "[%16llx]  " (hex), plus their trailing NULLs.  It's apparently
worked OK by dumb luck by putting the NULL either in the stack variable location
of the preceding "tcp" pointer or the "buf[BUFSIZE]" array (I guess???), which
as it turns out, is safe to do. 

Anyway I'll bump it up...

Thanks,
  Dave
 

 




More information about the Crash-utility mailing list