[PATCH] fix memory and descriptor leaks in grub_util_is_imsm

[PATCH] fix memory and descriptor leaks in grub_util_is_imsm

[Andrey Borzenkov]

Descriptor leak caused warning from later vgs invocation. Fix memory
leak (buffer was not always freed) while on it.

Signed-off-by: Andrey Borzenkov <arvidjaar@gmail.com>

---
 util/getroot.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/util/getroot.c b/util/getroot.c
index 2ad8a55..b30a0d8 100644
--- a/util/getroot.c
+++ b/util/getroot.c
@@ -1455,10 +1455,12 @@ out:
 static int
 grub_util_is_imsm (const char *os_dev)
 {
-  int try;
+  int retry;
+  int is_imsm = 0;
+  int container_seen = 0;
   const char *dev = os_dev;
 
-  for (try = 0; try < 2; try++)
+  do
     {
       char *argv[5];
       int fd;
@@ -1467,6 +1469,8 @@ grub_util_is_imsm (const char *os_dev)
       char *buf = NULL;
       size_t len = 0;
 
+      retry = 0; /* We'll do one more pass if device is part of container */
+
       /* execvp has inconvenient types, hence the casts.  None of these
 	 strings will actually be modified.  */
       argv[0] = (char *) "mdadm";
@@ -1499,7 +1503,8 @@ grub_util_is_imsm (const char *os_dev)
 
       while (getline (&buf, &len, mdadm) > 0)
 	{
-	  if (strncmp (buf, "MD_CONTAINER=", sizeof ("MD_CONTAINER=") - 1) == 0)
+	  if (strncmp (buf, "MD_CONTAINER=", sizeof ("MD_CONTAINER=") - 1) == 0
+	      && !container_seen)
 	    {
 	      char *newdev, *ptr;
 	      newdev = xstrdup (buf + sizeof ("MD_CONTAINER=") - 1);
@@ -1508,31 +1513,27 @@ grub_util_is_imsm (const char *os_dev)
 	      ptr[1] = 0;
 	      grub_util_info ("Container of %s is %s", dev, newdev);
 	      dev = newdev;
-	      goto out;
+	      container_seen = retry = 1;
+	      break;
 	    }
 	  if (strncmp (buf, "MD_METADATA=imsm",
 		       sizeof ("MD_METADATA=imsm") - 1) == 0)
 	    {
-	      close (fd);
-	      waitpid (pid, NULL, 0);
+	      is_imsm = 1;
 	      grub_util_info ("%s is imsm", dev);	      
-	      if (dev != os_dev)
-		free ((void *) dev);
-	      return 1;
+	      break;
 	    }
 	}
 
       free (buf);
-
-      return 0;
-
-    out:
       close (fd);
       waitpid (pid, NULL, 0);
     }
+  while (retry);
+
   if (dev != os_dev)
     free ((void *) dev);
-  return 0;
+  return is_imsm;
 }
 #endif /* __linux__ */
 
-- 
tg: (e1a892d..) u/imsm_descriptor_leak (depends on: master)

Re: [PATCH] fix memory and descriptor leaks in grub_util_is_imsm

[Vladimir 'φ-coder/phcoder' Serbinenko]

Go ahead.
On 19.07.2013 13:53, Andrey Borzenkov wrote:
> Descriptor leak caused warning from later vgs invocation. Fix memory
> leak (buffer was not always freed) while on it.
>
> Signed-off-by: Andrey Borzenkov <arvidjaar@gmail.com>
>
> ---
>   util/getroot.c | 29 +++++++++++++++--------------
>   1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/util/getroot.c b/util/getroot.c
> index 2ad8a55..b30a0d8 100644
> --- a/util/getroot.c
> +++ b/util/getroot.c
> @@ -1455,10 +1455,12 @@ out:
>   static int
>   grub_util_is_imsm (const char *os_dev)
>   {
> -  int try;
> +  int retry;
> +  int is_imsm = 0;
> +  int container_seen = 0;
>     const char *dev = os_dev;
>
> -  for (try = 0; try < 2; try++)
> +  do
>       {
>         char *argv[5];
>         int fd;
> @@ -1467,6 +1469,8 @@ grub_util_is_imsm (const char *os_dev)
>         char *buf = NULL;
>         size_t len = 0;
>
> +      retry = 0; /* We'll do one more pass if device is part of container */
> +
>         /* execvp has inconvenient types, hence the casts.  None of these
>   	 strings will actually be modified.  */
>         argv[0] = (char *) "mdadm";
> @@ -1499,7 +1503,8 @@ grub_util_is_imsm (const char *os_dev)
>
>         while (getline (&buf, &len, mdadm) > 0)
>   	{
> -	  if (strncmp (buf, "MD_CONTAINER=", sizeof ("MD_CONTAINER=") - 1) == 0)
> +	  if (strncmp (buf, "MD_CONTAINER=", sizeof ("MD_CONTAINER=") - 1) == 0
> +	      && !container_seen)
>   	    {
>   	      char *newdev, *ptr;
>   	      newdev = xstrdup (buf + sizeof ("MD_CONTAINER=") - 1);
> @@ -1508,31 +1513,27 @@ grub_util_is_imsm (const char *os_dev)
>   	      ptr[1] = 0;
>   	      grub_util_info ("Container of %s is %s", dev, newdev);
>   	      dev = newdev;
> -	      goto out;
> +	      container_seen = retry = 1;
> +	      break;
>   	    }
>   	  if (strncmp (buf, "MD_METADATA=imsm",
>   		       sizeof ("MD_METADATA=imsm") - 1) == 0)
>   	    {
> -	      close (fd);
> -	      waitpid (pid, NULL, 0);
> +	      is_imsm = 1;
>   	      grub_util_info ("%s is imsm", dev);	
> -	      if (dev != os_dev)
> -		free ((void *) dev);
> -	      return 1;
> +	      break;
>   	    }
>   	}
>
>         free (buf);
> -
> -      return 0;
> -
> -    out:
>         close (fd);
>         waitpid (pid, NULL, 0);
>       }
> +  while (retry);
> +
>     if (dev != os_dev)
>       free ((void *) dev);
> -  return 0;
> +  return is_imsm;
>   }
>   #endif /* __linux__ */
>
>