Re: [Xen-devel] [PATCH] Fix random segfaults in with xm top

[Anthony Liguori <aliguori@xxxxxxxxxx>]

Jerone Young wrote:

> On Wed, 2005-11-23 at 17:40 -0600, Anthony Liguori wrote:
>  
>
> > Unfortunately, this patch introduces another potential segfault from an 
> > unterminated string.  See below for how to fix.
> >    
> Actually I believe this is not true as I am assigning a character and
> not a character string. You do not need to null terminate a character.
> Doesn't really matter both accomplish the same thing. Using the function
> is better anyway.
>  
But name is a string and is being accessed as a string :-)  You're 
allocating 1 byte for it so when printf attempts to print out the string 
it will overrun the buffer.
Regards,

Anthony Liguori

> > Jerone Young wrote:
> >
> >    
> >
> > > # HG changeset patch
> > > # User root@leaf1
> > > # Node ID 7ce2dfd820e39c7764f276a785415014a7954861
> > > # Parent  14d733e5e1d014e302d72fb78df1428ee08e3ce3
> > > * fix random segfaults in xentop by never returning null
> > > * remove xenstore transcations (not needed).
> > >
> > > diff -r 14d733e5e1d0 -r 7ce2dfd820e3 tools/xenstat/libxenstat/src/xenstat.c
> > > --- a/tools/xenstat/libxenstat/src/xenstat.c    Wed Nov 23 13:15:35 2005
> > > +++ b/tools/xenstat/libxenstat/src/xenstat.c    Wed Nov 23 19:17:11 2005
> > > @@ -702,19 +702,16 @@
> > > {
> > >         char path[80];
> > >         char *name;
> > > -       struct xs_transaction_handle *xstranshandle;
> > >
> > >         snprintf(path, sizeof(path),"/local/domain/%i/name", domain_id);
> > >         
> > > -       xstranshandle = xs_transaction_start(handle->xshandle);
> > > -       if (xstranshandle == NULL) {
> > > -               perror("Unable to get transcation handle from xenstore\n");
> > > -               exit(1); /* Change this */
> > > -       }
> > > -
> > > -       name = (char *) xs_read(handle->xshandle, xstranshandle, path, NULL);
> > > +       name = (char *) xs_read(handle->xshandle, NULL, path, NULL);
> > >         
> > > -       xs_transaction_end(handle->xshandle, xstranshandle, false);
> > > +       if (name == NULL)
> > > +       {
> > > +               name = (char *)malloc((size_t)sizeof(char));
> > > +               name[0] = ' ';
> > > +	}	 
> > >
> > >      
> > -               name = (char *)malloc((size_t)sizeof(char));
> > -               name[0] = ' ';
> > +               name = malloc(2);
> > +               name[0] = ' ';
> > +               name[1] = 0;
> >
> > Or better yet:
> >
> > -               name = (char *)malloc((size_t)sizeof(char));
> > -               name[0] = ' ';
> > +               name = strdup(" ");
> >
> > Regards,
> >
> > Anthony Liguori
> >
> >    
> >
> > >         return name;
> > > }       
> > >
> > >
> > > ------------------------------------------------------------------------
> > >
> > > _______________________________________________
> > > Xen-devel mailing list
> > > Xen-devel@xxxxxxxxxxxxxxxxxxx
> > > http://lists.xensource.com/xen-devel
> > >
> > >
> > >      
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxxxxxxxx
> > http://lists.xensource.com/xen-devel
> >
> >    
>  
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel