[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] mini-os: Fix memory leaks in blkfront, netfront, pcifront, etc.



Hi,

The return value of Xenbus routines xenbus_transaction_start(),
xenbus_printf(), xenbus_transaction_end(), etc.
is a pointer of error message.
This pointer should be passed to free() to release the allocated memory
when it is no longer needed.

Signed-off-by: Yu Zhiguo <yuzg@xxxxxxxxxxxxxx>

diff --git a/extras/mini-os/blkfront.c b/extras/mini-os/blkfront.c
--- a/extras/mini-os/blkfront.c
+++ b/extras/mini-os/blkfront.c
@@ -93,7 +93,7 @@
     char* message=NULL;
     struct blkif_sring *s;
     int retry=0;
-    char* msg;
+    char* msg = NULL;
     char* c;
     char* nodename = _nodename ? _nodename : "device/vbd/768";
 
@@ -129,6 +129,7 @@
     err = xenbus_transaction_start(&xbt);
     if (err) {
         printk("starting transaction\n");
+        free(err);
     }
 
     err = xenbus_printf(xbt, nodename, "ring-ref","%u",
@@ -159,6 +160,7 @@
 
 
     err = xenbus_transaction_end(xbt, 0, &retry);
+    if (err) free(err);
     if (retry) {
             goto again;
         printk("completing transaction\n");
@@ -167,7 +169,8 @@
     goto done;
 
 abort_transaction:
-    xenbus_transaction_end(xbt, 1, &retry);
+    free(err);
+    err = xenbus_transaction_end(xbt, 1, &retry);
     goto error;
 
 done:
@@ -238,6 +241,8 @@
     return dev;
 
 error:
+    free(msg);
+    free(err);
     free_blkfront(dev);
     return NULL;
 }
@@ -265,6 +270,7 @@
     state = xenbus_read_integer(path);
     while (err == NULL && state < XenbusStateClosing)
         err = xenbus_wait_for_state_change(path, &state, &dev->events);
+    if (err) free(err);
 
     if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) != 
NULL) {
         printk("shutdown_blkfront: error changing state to %d: %s\n",
@@ -272,8 +278,10 @@
         goto close;
     }
     state = xenbus_read_integer(path);
-    if (state < XenbusStateClosed)
-        xenbus_wait_for_state_change(path, &state, &dev->events);
+    if (state < XenbusStateClosed) {
+        err = xenbus_wait_for_state_change(path, &state, &dev->events);
+        if (err) free(err);
+    }
 
     if ((err = xenbus_switch_state(XBT_NIL, nodename, 
XenbusStateInitialising)) != NULL) {
         printk("shutdown_blkfront: error changing state to %d: %s\n",
@@ -286,6 +294,7 @@
         err = xenbus_wait_for_state_change(path, &state, &dev->events);
 
 close:
+    if (err) free(err);
     xenbus_unwatch_path_token(XBT_NIL, path, path);
 
     snprintf(path, sizeof(path), "%s/ring-ref", nodename);
diff --git a/extras/mini-os/console/xencons_ring.c 
b/extras/mini-os/console/xencons_ring.c
--- a/extras/mini-os/console/xencons_ring.c
+++ b/extras/mini-os/console/xencons_ring.c
@@ -203,7 +203,7 @@
     char* err;
     char* message=NULL;
     int retry=0;
-    char* msg;
+    char* msg = NULL;
     char nodename[256];
     char path[256];
     static int consfrontends = 1;
@@ -242,6 +242,7 @@
     err = xenbus_transaction_start(&xbt);
     if (err) {
         printk("starting transaction\n");
+        free(err);
     }
 
     err = xenbus_printf(xbt, nodename, "ring-ref","%u",
@@ -278,6 +279,7 @@
 
 
     err = xenbus_transaction_end(xbt, 0, &retry);
+    if (err) free(err);
     if (retry) {
             goto again;
         printk("completing transaction\n");
@@ -286,7 +288,8 @@
     goto done;
 
 abort_transaction:
-    xenbus_transaction_end(xbt, 1, &retry);
+    free(err);
+    err = xenbus_transaction_end(xbt, 1, &retry);
     goto error;
 
 done:
@@ -323,6 +326,8 @@
     return dev;
 
 error:
+    free(msg);
+    free(err);
     free_consfront(dev);
     return NULL;
 }
diff --git a/extras/mini-os/fbfront.c b/extras/mini-os/fbfront.c
--- a/extras/mini-os/fbfront.c
+++ b/extras/mini-os/fbfront.c
@@ -71,7 +71,7 @@
     char* message=NULL;
     struct xenkbd_page *s;
     int retry=0;
-    char* msg;
+    char* msg = NULL;
     char* nodename = _nodename ? _nodename : "device/vkbd/0";
     struct kbdfront_dev *dev;
 
@@ -102,6 +102,7 @@
     err = xenbus_transaction_start(&xbt);
     if (err) {
         printk("starting transaction\n");
+        free(err);
     }
 
     err = xenbus_printf(xbt, nodename, "page-ref","%u", virt_to_mfn(s));
@@ -124,11 +125,13 @@
 
     snprintf(path, sizeof(path), "%s/state", nodename);
     err = xenbus_switch_state(xbt, path, XenbusStateInitialised);
-    if (err)
+    if (err) {
         printk("error writing initialized: %s\n", err);
-
+        free(err);
+    }
 
     err = xenbus_transaction_end(xbt, 0, &retry);
+    if (err) free(err);
     if (retry) {
             goto again;
         printk("completing transaction\n");
@@ -137,7 +140,8 @@
     goto done;
 
 abort_transaction:
-    xenbus_transaction_end(xbt, 1, &retry);
+    free(err);
+    err = xenbus_transaction_end(xbt, 1, &retry);
     goto error;
 
 done:
@@ -186,6 +190,8 @@
 
     return dev;
 error:
+    free(msg);
+    free(err);
     free_kbdfront(dev);
     return NULL;
 }
@@ -247,6 +253,7 @@
     state = xenbus_read_integer(path);
     while (err == NULL && state < XenbusStateClosing)
         err = xenbus_wait_for_state_change(path, &state, &dev->events);
+    if (err) free(err);
 
     if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) != 
NULL) {
         printk("shutdown_kbdfront: error changing state to %d: %s\n",
@@ -254,8 +261,10 @@
         goto close_kbdfront;
     }
     state = xenbus_read_integer(path);
-    if (state < XenbusStateClosed)
-        xenbus_wait_for_state_change(path, &state, &dev->events);
+    if (state < XenbusStateClosed) {
+        err = xenbus_wait_for_state_change(path, &state, &dev->events);
+        if (err) free(err);
+    }
 
     if ((err = xenbus_switch_state(XBT_NIL, nodename, 
XenbusStateInitialising)) != NULL) {
         printk("shutdown_kbdfront: error changing state to %d: %s\n",
@@ -266,6 +275,7 @@
     //xenbus_wait_for_value(path, "2", &dev->events);
 
 close_kbdfront:
+    if (err) free(err);
     xenbus_unwatch_path_token(XBT_NIL, path, path);
 
     snprintf(path, sizeof(path), "%s/page-ref", nodename);
@@ -446,6 +456,7 @@
     err = xenbus_transaction_start(&xbt);
     if (err) {
         printk("starting transaction\n");
+        free(err);
     }
 
     err = xenbus_printf(xbt, nodename, "page-ref","%u", virt_to_mfn(s));
@@ -478,6 +489,7 @@
     }
 
     err = xenbus_transaction_end(xbt, 0, &retry);
+    if (err) free(err);
     if (retry) {
             goto again;
         printk("completing transaction\n");
@@ -486,7 +498,8 @@
     goto done;
 
 abort_transaction:
-    xenbus_transaction_end(xbt, 1, &retry);
+    free(err);
+    err = xenbus_transaction_end(xbt, 1, &retry);
     goto error;
 
 done:
@@ -539,6 +552,7 @@
     return dev;
 
 error:
+    free(err);
     free_fbfront(dev);
     return NULL;
 }
@@ -627,6 +641,7 @@
     state = xenbus_read_integer(path);
     while (err == NULL && state < XenbusStateClosing)
         err = xenbus_wait_for_state_change(path, &state, &dev->events);
+    if (err) free(err);
 
     if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) != 
NULL) {
         printk("shutdown_fbfront: error changing state to %d: %s\n",
@@ -634,8 +649,10 @@
         goto close_fbfront;
     }
     state = xenbus_read_integer(path);
-    if (state < XenbusStateClosed)
+    if (state < XenbusStateClosed) {
         xenbus_wait_for_state_change(path, &state, &dev->events);
+        if (err) free(err);
+    }
 
     if ((err = xenbus_switch_state(XBT_NIL, nodename, 
XenbusStateInitialising)) != NULL) {
         printk("shutdown_fbfront: error changing state to %d: %s\n",
@@ -646,6 +663,7 @@
     //xenbus_wait_for_value(path, "2", &dev->events);
 
 close_fbfront:
+    if (err) free(err);
     xenbus_unwatch_path_token(XBT_NIL, path, path);
 
     snprintf(path, sizeof(path), "%s/page-ref", nodename);
diff --git a/extras/mini-os/fs-front.c b/extras/mini-os/fs-front.c
--- a/extras/mini-os/fs-front.c
+++ b/extras/mini-os/fs-front.c
@@ -1103,6 +1103,7 @@
     err = xenbus_transaction_start(&xbt);
     if (err) {
         printk("starting transaction\n");
+        free(err);
     }
     
     err = xenbus_printf(xbt, 
@@ -1140,9 +1141,11 @@
     }
 
     err = xenbus_printf(xbt, nodename, "state", STATE_READY, 0xdeadbeef);
+    if (err) free(err);
 
     
     err = xenbus_transaction_end(xbt, 0, &retry);
+    if (err) free(err);
     if (retry) {
             goto again;
         printk("completing transaction\n");
@@ -1159,7 +1162,9 @@
     goto done;
 
 abort_transaction:
-    xenbus_transaction_end(xbt, 1, &retry);
+    free(err);
+    err = xenbus_transaction_end(xbt, 1, &retry);
+    if (err) free(err);
 
 done:
 
@@ -1189,7 +1194,8 @@
     sprintf(token, "fs-front-%d", import->import_id);
     /* The token will not be unique if multiple imports are inited */
     xenbus_watch_path_token(XBT_NIL, r_nodename, r_nodename, &events);
-    xenbus_wait_for_value(r_nodename, STATE_READY, &events);
+    err = xenbus_wait_for_value(r_nodename, STATE_READY, &events);
+    if (err) free(err);
     xenbus_unwatch_path_token(XBT_NIL, r_nodename, r_nodename);
     printk("Backend ready.\n");
    
diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c
--- a/extras/mini-os/netfront.c
+++ b/extras/mini-os/netfront.c
@@ -305,7 +305,7 @@
     struct netif_rx_sring *rxs;
     int retry=0;
     int i;
-    char* msg;
+    char* msg = NULL;
     char nodename[256];
     char path[256];
     struct netfront_dev *dev;
@@ -377,6 +377,7 @@
     err = xenbus_transaction_start(&xbt);
     if (err) {
         printk("starting transaction\n");
+        free(err);
     }
 
     err = xenbus_printf(xbt, nodename, "tx-ring-ref","%u",
@@ -413,6 +414,7 @@
     }
 
     err = xenbus_transaction_end(xbt, 0, &retry);
+    if (err) free(err);
     if (retry) {
             goto again;
         printk("completing transaction\n");
@@ -421,7 +423,8 @@
     goto done;
 
 abort_transaction:
-    xenbus_transaction_end(xbt, 1, &retry);
+    free(err);
+    err = xenbus_transaction_end(xbt, 1, &retry);
     goto error;
 
 done:
@@ -479,6 +482,8 @@
 
     return dev;
 error:
+    free(msg);
+    free(err);
     free_netfront(dev);
     return NULL;
 }
@@ -521,6 +526,7 @@
     state = xenbus_read_integer(path);
     while (err == NULL && state < XenbusStateClosing)
         err = xenbus_wait_for_state_change(path, &state, &dev->events);
+    if (err) free(err);
 
     if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) != 
NULL) {
         printk("shutdown_netfront: error changing state to %d: %s\n",
@@ -528,8 +534,10 @@
         goto close;
     }
     state = xenbus_read_integer(path);
-    if (state < XenbusStateClosed)
-        xenbus_wait_for_state_change(path, &state, &dev->events);
+    if (state < XenbusStateClosed) {
+        err = xenbus_wait_for_state_change(path, &state, &dev->events);
+        if (err) free(err);
+    }
 
     if ((err = xenbus_switch_state(XBT_NIL, nodename, 
XenbusStateInitialising)) != NULL) {
         printk("shutdown_netfront: error changing state to %d: %s\n",
@@ -542,6 +550,7 @@
         err = xenbus_wait_for_state_change(path, &state, &dev->events);
 
 close:
+    if (err) free(err);
     xenbus_unwatch_path_token(XBT_NIL, path, path);
 
     snprintf(path, sizeof(path), "%s/tx-ring-ref", nodename);
diff --git a/extras/mini-os/pcifront.c b/extras/mini-os/pcifront.c
--- a/extras/mini-os/pcifront.c
+++ b/extras/mini-os/pcifront.c
@@ -181,6 +181,7 @@
     err = xenbus_transaction_start(&xbt);
     if (err) {
         printk("starting transaction\n");
+        free(err);
     }
 
     err = xenbus_printf(xbt, nodename, "pci-op-ref","%u",
@@ -210,6 +211,7 @@
     }
 
     err = xenbus_transaction_end(xbt, 0, &retry);
+    if (err) free(err);
     if (retry) {
             goto again;
         printk("completing transaction\n");
@@ -218,7 +220,8 @@
     goto done;
 
 abort_transaction:
-    xenbus_transaction_end(xbt, 1, &retry);
+    free(err);
+    err = xenbus_transaction_end(xbt, 1, &retry);
     goto error;
 
 done:
@@ -268,6 +271,7 @@
     return dev;
 
 error:
+    free(err);
     free_pcifront(dev);
     return NULL;
 }
@@ -341,6 +345,7 @@
     state = xenbus_read_integer(path);
     while (err == NULL && state < XenbusStateClosing)
         err = xenbus_wait_for_state_change(path, &state, &dev->events);
+    if (err) free(err);
 
     if ((err = xenbus_switch_state(XBT_NIL, nodename, XenbusStateClosed)) != 
NULL) {
         printk("shutdown_pcifront: error changing state to %d: %s\n",
@@ -348,8 +353,10 @@
         goto close_pcifront;
     }
     state = xenbus_read_integer(path);
-    if (state < XenbusStateClosed)
-        xenbus_wait_for_state_change(path, &state, &dev->events);
+    if (state < XenbusStateClosed) {
+        err = xenbus_wait_for_state_change(path, &state, &dev->events);
+        free(err);
+    }
 
     if ((err = xenbus_switch_state(XBT_NIL, nodename, 
XenbusStateInitialising)) != NULL) {
         printk("shutdown_pcifront: error changing state to %d: %s\n",
@@ -362,6 +369,7 @@
         err = xenbus_wait_for_state_change(path, &state, &dev->events);
 
 close_pcifront:
+    if (err) free(err);
     xenbus_unwatch_path_token(XBT_NIL, path, path);
 
     snprintf(path, sizeof(path), "%s/info-ref", nodename);
diff --git a/extras/mini-os/xenbus/xenbus.c b/extras/mini-os/xenbus/xenbus.c
--- a/extras/mini-os/xenbus/xenbus.c
+++ b/extras/mini-os/xenbus/xenbus.c
@@ -135,7 +135,8 @@
 
     do {
         if (xbt == XBT_NIL) {
-            xenbus_transaction_start(&xbt);
+            msg = xenbus_transaction_start(&xbt);
+            if (msg) goto exit;
             xbt_flag = 1;
         }
 


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.