Jerone Young
2005-Nov-23 18:18 UTC
[Xen-devel] [PATCH] Fix random segfaults in with xm top
This fixes a small race between when a domain is created and when xentop tries to read it''s info from the xenstore. Instead of sending a NULL pointer for the name the name will be displayed as a '' '' until the next refresh where xentop will then properly display the name. Signed-off-by: Jerone Young <jyoung5@us.ibm.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony Liguori
2005-Nov-23 23:40 UTC
Re: [Xen-devel] [PATCH] Fix random segfaults in with xm top
Unfortunately, this patch introduces another potential segfault from an unterminated string. See below for how to fix. 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@lists.xensource.com >http://lists.xensource.com/xen-devel > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jerone Young
2005-Nov-28 19:16 UTC
Re: [Xen-devel] [PATCH] Fix random segfaults in with xm top
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.> > 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@lists.xensource.com > >http://lists.xensource.com/xen-devel > > > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony Liguori
2005-Nov-28 19:31 UTC
Re: [Xen-devel] [PATCH] Fix random segfaults in with xm top
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@lists.xensource.com >>>http://lists.xensource.com/xen-devel >>> >>> >>> >>> >>_______________________________________________ >>Xen-devel mailing list >>Xen-devel@lists.xensource.com >>http://lists.xensource.com/xen-devel >> >> >> > > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel