[PATCH]: grub: Exliminate fixed length string buffers in child and devalias iterators.

[David Miller]

I did a lot of extensive research, and had some conversations with a
few openboot engineers, to determine the top-end values for properties
and path lengths.

Using that I added some defines and got rid of the various
fixed size stack lock buffers used throughout the openfirmware
code.  We now use dynamic allocation instead, and all of those
ugly "XXX" comments can be removed.

Finally, this works around a long standing bug in OpenBoot wherein
device alias properties are not NULL terminated.  We allocate
an extra byte and explicitly put a NULL character there in that
extra byte at the end.

2009-04-19  David S. Miller  <address@hidden>

        * include/grub/ieee1275/ieee1275.h (IEEE1275_MAX_PROP_LEN,
        IEEE1275_MAX_PATH_LEN): Define.
        * kern/ieee1275/openfw.c (grub_children_iterate): Dynamically
        allocate 'childtype', 'childpath', 'childname', and 'fullname'.
        (grub_devalias_iterate): Dynamically allocate 'aliasname' and
        'devtype'.  Explicitly NULL terminate devalias expansion.
---
 include/grub/ieee1275/ieee1275.h |    3 ++
 kern/ieee1275/openfw.c           |   64 ++++++++++++++++++++++++++++++++-----
 2 files changed, 58 insertions(+), 9 deletions(-)

diff --git a/include/grub/ieee1275/ieee1275.h b/include/grub/ieee1275/ieee1275.h
index 3b5139e..ed02f52 100644
--- a/include/grub/ieee1275/ieee1275.h
+++ b/include/grub/ieee1275/ieee1275.h
@@ -39,6 +39,9 @@ struct grub_ieee1275_mem_region
   unsigned int size;
 };
 
+#define IEEE1275_MAX_PROP_LEN  8192
+#define IEEE1275_MAX_PATH_LEN  256
+
 #ifndef IEEE1275_CALL_ENTRY_FN
 #define IEEE1275_CALL_ENTRY_FN(args) (*grub_ieee1275_entry_fn) (args)
 #endif
diff --git a/kern/ieee1275/openfw.c b/kern/ieee1275/openfw.c
index d70c3ba..83bac65 100644
--- a/kern/ieee1275/openfw.c
+++ b/kern/ieee1275/openfw.c
@@ -38,6 +38,8 @@ grub_children_iterate (char *devpath,
 {
   grub_ieee1275_phandle_t dev;
   grub_ieee1275_phandle_t child;
+  char *childtype, *childpath;
+  char *childname, *fullname;
 
   if (grub_ieee1275_finddevice (devpath, &dev))
     return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "Unknown device");
@@ -45,13 +47,33 @@ grub_children_iterate (char *devpath,
   if (grub_ieee1275_child (dev, &child))
     return grub_error (GRUB_ERR_BAD_DEVICE, "Device has no children");
 
+  childtype = grub_malloc (IEEE1275_MAX_PROP_LEN);
+  if (!childtype)
+    return grub_errno;
+  childpath = grub_malloc (IEEE1275_MAX_PATH_LEN);
+  if (!childpath)
+    {
+      grub_free (childtype);
+      return grub_errno;
+    }
+  childname = grub_malloc (IEEE1275_MAX_PROP_LEN);
+  if (!childname)
+    {
+      grub_free (childpath);
+      grub_free (childtype);
+      return grub_errno;
+    }
+  fullname = grub_malloc (IEEE1275_MAX_PATH_LEN);
+  if (!fullname)
+    {
+      grub_free (childname);
+      grub_free (childpath);
+      grub_free (childtype);
+      return grub_errno;
+    }
+
   do
     {
-      /* XXX: Don't use hardcoded path lengths.  */
-      char childtype[64];
-      char childpath[64];
-      char childname[64];
-      char fullname[64];
       struct grub_ieee1275_devalias alias;
       grub_ssize_t actual;
 
@@ -76,6 +98,11 @@ grub_children_iterate (char *devpath,
     }
   while (grub_ieee1275_peer (child, &child));
 
+  grub_free (fullname);
+  grub_free (childname);
+  grub_free (childpath);
+  grub_free (childtype);
+
   return 0;
 }
 
@@ -85,13 +112,23 @@ grub_err_t
 grub_devalias_iterate (int (*hook) (struct grub_ieee1275_devalias *alias))
 {
   grub_ieee1275_phandle_t aliases;
-  char aliasname[32];
+  char *aliasname, *devtype;
   grub_ssize_t actual;
   struct grub_ieee1275_devalias alias;
 
   if (grub_ieee1275_finddevice ("/aliases", &aliases))
     return -1;
 
+  aliasname = grub_malloc (IEEE1275_MAX_PROP_LEN);
+  if (!aliasname)
+    return grub_errno;
+  devtype = grub_malloc (IEEE1275_MAX_PROP_LEN);
+  if (!devtype)
+    {
+      grub_free (aliasname);
+      return grub_errno;
+    }
+
   /* Find the first property.  */
   aliasname[0] = '\0';
 
@@ -100,8 +137,6 @@ grub_devalias_iterate (int (*hook) (struct 
grub_ieee1275_devalias *alias))
       grub_ieee1275_phandle_t dev;
       grub_ssize_t pathlen;
       char *devpath;
-      /* XXX: This should be large enough for any possible case.  */
-      char devtype[64];
 
       grub_dprintf ("devalias", "devalias name = %s\n", aliasname);
 
@@ -111,9 +146,17 @@ grub_devalias_iterate (int (*hook) (struct 
grub_ieee1275_devalias *alias))
       if (!grub_strcmp (aliasname, "name"))
        continue;
 
+      /* Sun's OpenBoot often doesn't zero terminate the device alias
+        strings, so we will add a NULL byte at the end explicitly.  */
+      pathlen += 1;
+
       devpath = grub_malloc (pathlen);
       if (! devpath)
-       return grub_errno;
+       {
+         grub_free (devtype);
+         grub_free (aliasname);
+         return grub_errno;
+       }
 
       if (grub_ieee1275_get_property (aliases, aliasname, devpath, pathlen,
                                      &actual))
@@ -121,6 +164,7 @@ grub_devalias_iterate (int (*hook) (struct 
grub_ieee1275_devalias *alias))
          grub_dprintf ("devalias", "get_property (%s) failed\n", aliasname);
          goto nextprop;
        }
+      devpath [actual] = '\0';
 
       if (grub_ieee1275_finddevice (devpath, &dev))
        {
@@ -144,6 +188,8 @@ nextprop:
       grub_free (devpath);
     }
 
+  grub_free (devtype);
+  grub_free (aliasname);
   return 0;
 }
 
-- 
1.6.2.3