All of lore.kernel.org
 help / color / mirror / Atom feed
* Patch for getroot.c
@ 2011-02-11 19:58 Seth Goldberg
  2011-03-26 22:24 ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 1 reply; 2+ messages in thread
From: Seth Goldberg @ 2011-02-11 19:58 UTC (permalink / raw)
  To: grub-devel

Hi,

   When testing with UMEM_DEBUG set to default on Solaris, I discovered that 
find_root_device_from_libzfs() was maintaining a pointer to state that was 
freed when zpool_close() was called (specifically, the device path string, 
retrieved from an nvlist).  The path here fixes that and make this function a 
bit more bulletproof by testing for NULLs.  (Note that this returns memory 
that is not freed by the util programs that use it, but that's not unique to 
this patch -- all strings from grub_guess_root_device() aren't freed after 
they're used.)

=== modified file 'grub-core/kern/emu/getroot.c'
--- grub-core/kern/emu/getroot.c	2011-01-12 02:57:53 +0000
+++ grub-core/kern/emu/getroot.c	2011-02-11 19:48:23 +0000
@@ -185,7 +185,7 @@
  static char *
  find_root_device_from_libzfs (const char *dir, struct stat *sbp)
  {
-  char *device;
+  char *device = NULL;
    char *poolname;
    char *poolfs;

@@ -224,9 +224,13 @@
  	if (nvlist_lookup_string (children[i], "path", &device) != 0)
  	  error (1, errno, "nvlist_lookup_string (\"path\")");

+        grub_dprintf("zfs", "device -> %s\n", device);
+
  	struct stat st;
-	if (stat (device, &st) == 0)
+	if (stat (device, &st) == 0) {
+          device = grub_strdup(device);
  	  break;
+        }

  	device = NULL;
        }
@@ -240,12 +244,15 @@

  #if defined(__sun__)
    /* If the device is a /dev/dsk path, convert it into a /dev/rdsk one */
-  if (strncmp(device, "/dev/dsk/", 9) == 0) {
-    device = xasprintf("/dev/rdsk/%s", device + 9);
+  if (device != NULL && strncmp(device, "/dev/dsk/", 9) == 0) {
+    char *newdevice = xasprintf("/dev/rdsk/%s", device + 9);
+    grub_free(device);
+    device = newdevice;
    }
  #endif

-  grub_util_info("zfs path = %s", device);
+  if (device != NULL)
+    grub_util_info("zfs path = %s", device);

    return device;
  }



^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: Patch for getroot.c
  2011-02-11 19:58 Patch for getroot.c Seth Goldberg
@ 2011-03-26 22:24 ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 0 replies; 2+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2011-03-26 22:24 UTC (permalink / raw)
  To: grub-devel

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

On 11.02.2011 20:58, Seth Goldberg wrote:
> Hi,
>
>   When testing with UMEM_DEBUG set to default on Solaris, I discovered
> that find_root_device_from_libzfs() was maintaining a pointer to state
> that was freed when zpool_close() was called (specifically, the device
> path string, retrieved from an nvlist).  The path here fixes that and
> make this function a bit more bulletproof by testing for NULLs.  (Note
> that this returns memory that is not freed by the util programs that
> use it, but that's not unique to this patch -- all strings from
> grub_guess_root_device() aren't freed after they're used.)
>
Applied with some changes. Patch for the parts we don't have is
obviously skipped
> === modified file 'grub-core/kern/emu/getroot.c'
> --- grub-core/kern/emu/getroot.c    2011-01-12 02:57:53 +0000
> +++ grub-core/kern/emu/getroot.c    2011-02-11 19:48:23 +0000
> @@ -185,7 +185,7 @@
>  static char *
>  find_root_device_from_libzfs (const char *dir, struct stat *sbp)
>  {
> -  char *device;
> +  char *device = NULL;
>    char *poolname;
>    char *poolfs;
>
> @@ -224,9 +224,13 @@
>      if (nvlist_lookup_string (children[i], "path", &device) != 0)
>        error (1, errno, "nvlist_lookup_string (\"path\")");
>
> +        grub_dprintf("zfs", "device -> %s\n", device);
> +
>      struct stat st;
> -    if (stat (device, &st) == 0)
> +    if (stat (device, &st) == 0) {
> +          device = grub_strdup(device);
>        break;
> +        }
>
>      device = NULL;
>        }
> @@ -240,12 +244,15 @@
>
>  #if defined(__sun__)
>    /* If the device is a /dev/dsk path, convert it into a /dev/rdsk
> one */
> -  if (strncmp(device, "/dev/dsk/", 9) == 0) {
> -    device = xasprintf("/dev/rdsk/%s", device + 9);
> +  if (device != NULL && strncmp(device, "/dev/dsk/", 9) == 0) {
> +    char *newdevice = xasprintf("/dev/rdsk/%s", device + 9);
> +    grub_free(device);
> +    device = newdevice;
>    }
>  #endif
>
> -  grub_util_info("zfs path = %s", device);
> +  if (device != NULL)
> +    grub_util_info("zfs path = %s", device);
>
>    return device;
>  }
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2011-03-26 22:24 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-11 19:58 Patch for getroot.c Seth Goldberg
2011-03-26 22:24 ` Vladimir 'φ-coder/phcoder' Serbinenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.