From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1MOw5b-00021o-HS for mharc-grub-devel@gnu.org; Thu, 09 Jul 2009 12:03:15 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MOw5Y-0001yu-CO for grub-devel@gnu.org; Thu, 09 Jul 2009 12:03:12 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MOw5T-0001vY-9h for grub-devel@gnu.org; Thu, 09 Jul 2009 12:03:11 -0400 Received: from [199.232.76.173] (port=50776 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MOw5S-0001vG-S5 for grub-devel@gnu.org; Thu, 09 Jul 2009 12:03:07 -0400 Received: from aybabtu.com ([69.60.117.155]:56924) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1MOw5R-0002e1-Rl for grub-devel@gnu.org; Thu, 09 Jul 2009 12:03:06 -0400 Received: from [192.168.10.10] (helo=thorin) by aybabtu.com with esmtp (Exim 4.69) (envelope-from ) id 1MOv0q-0006N9-GH; Thu, 09 Jul 2009 16:54:17 +0200 Received: from rmh by thorin with local (Exim 4.69) (envelope-from ) id 1MOw5N-0001li-DP; Thu, 09 Jul 2009 18:03:01 +0200 Date: Thu, 9 Jul 2009 18:03:01 +0200 From: Robert Millan To: grub-devel@gnu.org Message-ID: <20090709160301.GA29422@thorin> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Organization: free as in freedom X-Message-Flag: Worried about Outlook viruses? Switch to Thunderbird! www.mozilla.com/thunderbird X-Debbugs-No-Ack: true User-Agent: Mutt/1.5.18 (2008-05-17) X-detected-operating-system: by monty-python.gnu.org: GNU/Linux 2.6 (newer, 1) Cc: "David S. Miller" Subject: OLPC regression, ofdisk stops working X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GRUB 2 List-Id: The development of GRUB 2 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Jul 2009 16:03:13 -0000 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 > > 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."