Hi,
There is something wrong with r2132, now childtype is a pointer, so
sizeof childtype == 4, the name would be truncated to 4 characters.
diff --git a/kern/ieee1275/openfw.c b/kern/ieee1275/openfw.c
index e7979f4..42d9278 100644
--- a/kern/ieee1275/openfw.c
+++ b/kern/ieee1275/openfw.c
@@ -78,15 +78,15 @@ grub_children_iterate (char *devpath,
grub_ssize_t actual;
if (grub_ieee1275_get_property (child, "device_type", childtype,
- sizeof childtype, &actual))
+ IEEE1275_MAX_PROP_LEN, &actual))
continue;
- if (grub_ieee1275_package_to_path (child, childpath, sizeof childpath,
- &actual))
+ if (grub_ieee1275_package_to_path (child, childpath,
+ IEEE1275_MAX_PATH_LEN, &actual))
continue;
if (grub_ieee1275_get_property (child, "name", childname,
- sizeof childname, &actual))
+ IEEE1275_MAX_PATH_LEN, &actual))
continue;
grub_sprintf (fullname, "%s/%s", devpath, childname);
On Fri, Jul 10, 2009 at 12:03 AM, Robert Millan<r...@aybabtu.com> wrote:
>
> Hi,
>
> I got completely puzzled at this one. Turns out r2132 broke ofdisk on
> OLPC. But I don't see anything wrong in this commit.
>
> I isolated the change that causes this breakage, and it's very confusing.
> It turns out that allocating devtype in the heap instead of the stack
> causes its result to be truncated to 4 bytes (+ null terminator).
>
> I'm not sure what can we do about this. If we're certain it's an OFW
> bug, perhaps we could workaround it by comparing only the first 4 bytes
> of the result. This worked for me:
>
> - if (! grub_strcmp (alias->type, "block"))
> + if (! grub_strncmp (alias->type, "bloc", 4))
>
> (but using the existing "workaround framework")
>
> but it's scary. I don't know if this 4-byte limit is consistent, or
> will differ arbitrarily. Maybe we could issue a warning or so, I don't
> know.
>
> Is there someone else (Bean?) who can reproduce this problem? Does it
> fail in the same way?
>
> On Wed, Apr 22, 2009 at 09:46:55AM +0000, David S. Miller wrote:
>> Revision: 2132
>> http://svn.sv.gnu.org/viewvc/?view=rev&root=grub&revision=2132
>> Author: davem
>> Date: 2009-04-22 09:46:54 +0000 (Wed, 22 Apr 2009)
>> Log Message:
>> -----------
>> * 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.
>>
>> Modified Paths:
>> --------------
>> trunk/grub2/ChangeLog
>> trunk/grub2/include/grub/ieee1275/ieee1275.h
>> trunk/grub2/kern/ieee1275/openfw.c
>>
>> Modified: trunk/grub2/ChangeLog
>> ===================================================================
>> --- trunk/grub2/ChangeLog 2009-04-22 09:45:43 UTC (rev 2131)
>> +++ trunk/grub2/ChangeLog 2009-04-22 09:46:54 UTC (rev 2132)
>> @@ -3,6 +3,13 @@
>> * kern/ieee1275/mmap.c (grub_machine_mmap_iterate): If size_cells
>> is larger than address_cells, use that value for address_cells too.
>>
>> + * 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.
>> +
>> 2009-04-19 Vladimir Serbinenko <phco...@gmail.com>
>>
>> Correct GPT definition
>>
>> Modified: trunk/grub2/include/grub/ieee1275/ieee1275.h
>> ===================================================================
>> --- trunk/grub2/include/grub/ieee1275/ieee1275.h 2009-04-22 09:45:43
>> UTC (rev 2131)
>> +++ trunk/grub2/include/grub/ieee1275/ieee1275.h 2009-04-22 09:46:54
>> UTC (rev 2132)
>> @@ -39,6 +39,9 @@
>> 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
>>
>> Modified: trunk/grub2/kern/ieee1275/openfw.c
>> ===================================================================
>> --- trunk/grub2/kern/ieee1275/openfw.c 2009-04-22 09:45:43 UTC (rev
>> 2131)
>> +++ trunk/grub2/kern/ieee1275/openfw.c 2009-04-22 09:46:54 UTC (rev
>> 2132)
>> @@ -38,6 +38,8 @@
>> {
>> 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 @@
>> 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 @@
>> }
>> 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_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_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 @@
>> 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_dprintf ("devalias", "get_property (%s) failed\n", aliasname);
>> goto nextprop;
>> }
>> + devpath [actual] = '\0';
>>
>> if (grub_ieee1275_finddevice (devpath, &dev))
>> {
>> @@ -144,6 +188,8 @@
>> grub_free (devpath);
>> }
>>
>> + grub_free (devtype);
>> + grub_free (aliasname);
>> return 0;
>> }
>>
>>
>>
>>
>>
>
> --
> Robert Millan
>
> The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
> how) you may access your data; but nobody's threatening your freedom: we
> still allow you to remove your data and not access it at all."
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
--
Bean
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
