[PATCH] Fix random segfaults in with xm top

[PATCH] Fix random segfaults in with xm top

[Jerone Young]

[-- Attachment #1: Type: text/plain, Size: 323 bytes --]

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>

[-- Attachment #2: xenstat_fix.diff --]
[-- Type: text/x-patch, Size: 1139 bytes --]

# 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] = ' ';
+	}	
 
 	return name;
 }	

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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

[Anthony Liguori]

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

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

[Jerone Young]

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
> 

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

[Anthony Liguori]

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