Fix random segfaults in with xm top

PATCH In review Jerone Young 2 participants 4 messages Nov 23, 2005 – Nov 28, 2005

Share & tools

Jerone Young

author

2005-11-23 18:18 UTC #0 lore raw

PATCH 1/1 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 <[email protected]>

# 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).

tools/xenstat/libxenstat/src/xenstat.c +6 -9

diff -r 14d733e5e1d0 -r 7ce2dfd820e3 tools/xenstat/libxenstat/src/xenstat.c
@@ -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;
 }

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Anthony Liguori

2005-11-23 23:40 UTC #1 lore raw

Unfortunately, this patch introduces another potential segfault from an unterminated string. See below for how to fix.

Quoted 25 lines · Jerone Young

# 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
tools/xenstat/libxenstat/src/xenstat.c +0 -8
@@ -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);

tools/xenstat/libxenstat/src/xenstat.c line 716

Quoted 10 lines

tools/xenstat/libxenstat/src/xenstat.c +6 -1

+	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

Quoted 14 lines

tools/xenstat/libxenstat/src/xenstat.c +0 -0
 	return name;
}
------------------------------------------------------------------------
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Jerone Young

author

2005-11-28 19:16 UTC #2 lore raw

Quoted 2 lines · Anthony Liguori

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.

Quoted 75 lines

Quoted 35 lines · Jerone Young
# 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
tools/xenstat/libxenstat/src/xenstat.c +6 -9
@@ -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] = ' ';
+	}	 
 
 
(quoted diff) +3 -2
-		name = (char *)malloc((size_t)sizeof(char));
-		name[0] = ' ';
+               name = malloc(2);
+		name[0] = ' ';
+		name[1] = 0;
Or better yet:
(quoted diff) +1 -2
-		name = (char *)malloc((size_t)sizeof(char));
-		name[0] = ' ';
+		name = strdup(" ");
Regards,
Anthony Liguori
Quoted 6 lines
tools/xenstat/libxenstat/src/xenstat.c +0 -0
 	return name;
}
Quoted 8 lines
------------------------------------------------------------------------
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Anthony Liguori

2005-11-28 19:31 UTC #3 lore raw

Quoted 13 lines · Jerone Young

Quoted 3 lines · Anthony Liguori
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

Quoted 86 lines

Quoted 82 lines
Quoted 37 lines · Jerone Young
# 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
tools/xenstat/libxenstat/src/xenstat.c +6 -9
@@ -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] = ' ';
+	}	 
 
 
 
(quoted diff) +3 -2
-		name = (char *)malloc((size_t)sizeof(char));
-		name[0] = ' ';
+               name = malloc(2);
+		name[0] = ' ';
+		name[1] = 0;
Or better yet:
(quoted diff) +1 -2
-		name = (char *)malloc((size_t)sizeof(char));
-		name[0] = ' ';
+		name = strdup(" ");
Regards,
Anthony Liguori
Quoted 6 lines
	return name;
}	
Quoted 10 lines
------------------------------------------------------------------------
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel