[PATCH] tools/xenpaging: fix bug of Segmentation fault

[PATCH] tools/xenpaging: fix bug of Segmentation fault

[Yu Zhiguo]

Segmentation fault occurs in two situations:
1. argc is less than 3
2. xenpaging_init() fault

Signed-off-by: Yu Zhiguo <yuzg@cn.fujitsu.com>

diff --git a/tools/xenpaging/xenpaging.c b/tools/xenpaging/xenpaging.c
--- a/tools/xenpaging/xenpaging.c
+++ b/tools/xenpaging/xenpaging.c
@@ -212,6 +212,9 @@
 {
     int rc;
 
+    if ( paging == NULL )
+        return 0;
+
     /* Tear down domain paging in Xen */
     rc = xc_mem_event_disable(paging->xc_handle, paging->mem_event.domain_id);
     if ( rc != 0 )
@@ -447,20 +450,29 @@
 
 int main(int argc, char *argv[])
 {
-    domid_t domain_id = atoi(argv[1]);
-    int num_pages = atoi(argv[2]);
+    domid_t domain_id;
+    int num_pages;
     xenpaging_t *paging;
-    xenpaging_victim_t victims[num_pages];
+    xenpaging_victim_t *victims;
     mem_event_request_t req;
     mem_event_response_t rsp;
     int i;
-    int rc;
+    int rc = -1, rc1;
 
     int open_flags = O_CREAT | O_TRUNC | O_RDWR;
     mode_t open_mode = S_IRUSR | S_IRGRP | S_IROTH | S_IWUSR | S_IWGRP | S_IWOTH;
     char filename[80];
     int fd;
 
+    if ( argc != 3 ) {
+        fprintf(stderr, "Usage: %s <domain_id> <num_pages>\n", argv[0]);
+	return -1;
+    }
+    domain_id = atoi(argv[1]);
+    num_pages = atoi(argv[2]);
+
+    victims = calloc(num_pages, sizeof(xenpaging_victim_t));
+
     /* Open file */
     sprintf(filename, "page_cache_%d", domain_id);
     fd = open(filename, open_flags, open_mode);
@@ -586,15 +598,17 @@
     }
 
  out:
+    free(victims);
+
     /* Tear down domain paging */
-    rc = xenpaging_teardown(paging);
-    if ( rc != 0 )
-    {
+    rc1 = xenpaging_teardown(paging);
+    if ( rc1 != 0 )
         ERROR("Error tearing down paging");
-        exit(1);
-    }
 
-    return 0;
+    if ( rc == 0 )
+        rc = rc1;
+
+    return rc;
 }

Re: [PATCH] tools/xenpaging: fix bug of Segmentation fault

[Patrick Colp]

Great catch, thanks! Just a couple minor nitpicks, if this patch
hasn't been applied yet.

For the line:

 +    int rc = -1, rc1;

could this instead be:

+    int rc = -1;
+    int rc1;


And for:

+    if ( argc != 3 ) {

would it be possible to move the { down onto it's own line?

These changes would make it fit in with the rest of the source better.


Thanks,
Patrick


On 10 February 2010 21:57, Yu Zhiguo <yuzg@cn.fujitsu.com> wrote:
> Segmentation fault occurs in two situations:
> 1. argc is less than 3
> 2. xenpaging_init() fault
>
> Signed-off-by: Yu Zhiguo <yuzg@cn.fujitsu.com>
>
> diff --git a/tools/xenpaging/xenpaging.c b/tools/xenpaging/xenpaging.c
> --- a/tools/xenpaging/xenpaging.c
> +++ b/tools/xenpaging/xenpaging.c
> @@ -212,6 +212,9 @@
>  {
>     int rc;
>
> +    if ( paging == NULL )
> +        return 0;
> +
>     /* Tear down domain paging in Xen */
>     rc = xc_mem_event_disable(paging->xc_handle, paging->mem_event.domain_id);
>     if ( rc != 0 )
> @@ -447,20 +450,29 @@
>
>  int main(int argc, char *argv[])
>  {
> -    domid_t domain_id = atoi(argv[1]);
> -    int num_pages = atoi(argv[2]);
> +    domid_t domain_id;
> +    int num_pages;
>     xenpaging_t *paging;
> -    xenpaging_victim_t victims[num_pages];
> +    xenpaging_victim_t *victims;
>     mem_event_request_t req;
>     mem_event_response_t rsp;
>     int i;
> -    int rc;
> +    int rc = -1, rc1;
>
>     int open_flags = O_CREAT | O_TRUNC | O_RDWR;
>     mode_t open_mode = S_IRUSR | S_IRGRP | S_IROTH | S_IWUSR | S_IWGRP | S_IWOTH;
>     char filename[80];
>     int fd;
>
> +    if ( argc != 3 ) {
> +        fprintf(stderr, "Usage: %s <domain_id> <num_pages>\n", argv[0]);
> +       return -1;
> +    }
> +    domain_id = atoi(argv[1]);
> +    num_pages = atoi(argv[2]);
> +
> +    victims = calloc(num_pages, sizeof(xenpaging_victim_t));
> +
>     /* Open file */
>     sprintf(filename, "page_cache_%d", domain_id);
>     fd = open(filename, open_flags, open_mode);
> @@ -586,15 +598,17 @@
>     }
>
>  out:
> +    free(victims);
> +
>     /* Tear down domain paging */
> -    rc = xenpaging_teardown(paging);
> -    if ( rc != 0 )
> -    {
> +    rc1 = xenpaging_teardown(paging);
> +    if ( rc1 != 0 )
>         ERROR("Error tearing down paging");
> -        exit(1);
> -    }
>
> -    return 0;
> +    if ( rc == 0 )
> +        rc = rc1;
> +
> +    return rc;
>  }
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>