On 28.10.2013 09:43, Francesco Lavra wrote: > Hi, > The current code in grub-core/lib/fdt.c doesn't work for me (and I > suspect it doesn't work for anybody else either): GRUB just hangs when > trying to load a Linux kernel with an associated device tree. In fact, > the current code is still the first version of fdt.c which I sent to > the list (warning that it wasn't completely tested), and afterwards my > attempts to push a fixed code didn't get much attention. I've looked at wrong patch probably. Committed patch, thanks. > I noticed that Leif's tree in Launchpad still uses the external libfdt, > and this makes me think that the current version of fdt.c doesn't work > for him either. Now, with the port to 64-bit ARM in progress (which > supposedly will use our fdt files), I think it would good to get fdt.c > fixed as soon as possible, in order to avoid duplicated efforts. > So here goes another attempt at it, this time as a full blown patch. > > Francesco > > --- > ChangeLog | 4 ++ > grub-core/lib/fdt.c | 136 > ++++++++++++++++++++++++++++++++------------------- > 2 files changed, 89 insertions(+), 51 deletions(-) > > diff --git a/ChangeLog b/ChangeLog > index bdd2c80..ad30352 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,3 +1,7 @@ > +2013-10-28 Francesco Lavra <francescolavra...@gmail.com> > + > + * grub-core/lib/fdt.c: Fix miscellaneous bugs. > + > 2013-10-28 Vladimir Serbinenko <phco...@gmail.com> > > * grub-core/kern/emu/hostdisk.c (grub_util_check_file_presence): Use > diff --git a/grub-core/lib/fdt.c b/grub-core/lib/fdt.c > index 57528c5..9b77e1c 100644 > --- a/grub-core/lib/fdt.c > +++ b/grub-core/lib/fdt.c > @@ -43,6 +43,16 @@ > #define prop_entry_size(prop_len) \ > (3 * sizeof(grub_uint32_t) + ALIGN_UP(prop_len, sizeof(grub_uint32_t))) > > +#define SKIP_NODE_NAME(name, token, end) \ > + name = (char *) ((token) + 1); \ > + while (name < (char *) end) \ > + { \ > + if (!*name++) \ > + break; \ > + } \ > + token = (grub_uint32_t *) ALIGN_UP((grub_addr_t) (name), sizeof(*token)) > + > + > static grub_uint32_t *get_next_node (const void *fdt, char *node_name) > { > grub_uint32_t *end = (void *) struct_end (fdt); > @@ -50,9 +60,9 @@ static grub_uint32_t *get_next_node (const void *fdt, char > *node_name) > > if (node_name >= (char *) end) > return NULL; > - while (*node_name) > + while (*node_name++) > { > - if (++node_name >= (char *) end) > + if (node_name >= (char *) end) > return NULL; > } > token = (grub_uint32_t *) ALIGN_UP ((grub_addr_t) node_name, 4); > @@ -73,7 +83,8 @@ static grub_uint32_t *get_next_node (const void *fdt, char > *node_name) > case FDT_PROP: > /* Skip property token and following data (len, nameoff and property > value). */ > - token += 3 + grub_be_to_cpu32 (*(token + 1)); > + token += prop_entry_size(grub_be_to_cpu32(*(token + 1))) > + / sizeof(*token); > break; > case FDT_NOP: > token++; > @@ -116,12 +127,14 @@ static grub_uint32_t get_free_space (void *fdt) > > static int add_subnode (void *fdt, int parentoffset, const char *name) > { > - grub_uint32_t *begin = (void *) ((grub_addr_t) fdt > + grub_uint32_t *token = (void *) ((grub_addr_t) fdt > + grub_fdt_get_off_dt_struct(fdt) > + parentoffset); > grub_uint32_t *end = (void *) struct_end (fdt); > unsigned int entry_size = node_entry_size (name); > - grub_uint32_t *token = begin; > + char *node_name; > + > + SKIP_NODE_NAME(node_name, token, end); > > /* Insert the new subnode just after the properties of the parent node (if > any).*/ > @@ -132,28 +145,28 @@ static int add_subnode (void *fdt, int parentoffset, > const char *name) > switch (grub_be_to_cpu32(*token)) > { > case FDT_PROP: > - /* Skip len and nameoff. */ > - token += 2; > + /* Skip len, nameoff and property value. */ > + token += prop_entry_size(grub_be_to_cpu32(*(token + 1))) > + / sizeof(*token); > break; > case FDT_BEGIN_NODE: > case FDT_END_NODE: > goto insert; > case FDT_NOP: > + token++; > break; > default: > /* invalid token */ > return -1; > } > - token++; > } > insert: > - grub_memmove (token + entry_size, token, > + grub_memmove (token + entry_size / sizeof(*token), token, > (grub_addr_t) end - (grub_addr_t) token); > *token = grub_cpu_to_be32(FDT_BEGIN_NODE); > token[entry_size / sizeof(*token) - 2] = 0; /* padding bytes */ > grub_strcpy((char *) (token + 1), name); > - token += entry_size / sizeof(*token) - 1; > - *token = grub_cpu_to_be32(FDT_END_NODE); > + token[entry_size / sizeof(*token) - 1] = grub_cpu_to_be32(FDT_END_NODE); > return ((grub_addr_t) token - (grub_addr_t) fdt > - grub_fdt_get_off_dt_struct(fdt)); > } > @@ -175,29 +188,32 @@ static int rearrange_blocks (void *fdt, unsigned int > clearance) > > if ((grub_fdt_get_off_mem_rsvmap (fdt) == off_mem_rsvmap) > && (grub_fdt_get_off_dt_struct (fdt) == off_dt_struct)) > - { > - /* No need to allocate memory for a temporary FDT, just move the strings > - block if needed. */ > - if (grub_fdt_get_off_dt_strings (fdt) != off_dt_strings) > - grub_memmove(fdt_ptr + off_dt_strings, > - fdt_ptr + grub_fdt_get_off_dt_strings (fdt), > - grub_fdt_get_size_dt_strings (fdt)); > - return 0; > - } > + { > + /* No need to allocate memory for a temporary FDT, just move the > strings > + block if needed. */ > + if (grub_fdt_get_off_dt_strings (fdt) != off_dt_strings) > + { > + grub_memmove(fdt_ptr + off_dt_strings, > + fdt_ptr + grub_fdt_get_off_dt_strings (fdt), > + grub_fdt_get_size_dt_strings (fdt)); > + grub_fdt_set_off_dt_strings (fdt, off_dt_strings); > + } > + return 0; > + } > tmp_fdt = grub_malloc (grub_fdt_get_totalsize (fdt)); > if (!tmp_fdt) > return -1; > grub_memcpy (tmp_fdt + off_mem_rsvmap, > - fdt_ptr + grub_fdt_get_off_mem_rsvmap (fdt), > - get_mem_rsvmap_size (fdt)); > + fdt_ptr + grub_fdt_get_off_mem_rsvmap (fdt), > + get_mem_rsvmap_size (fdt)); > grub_fdt_set_off_mem_rsvmap (fdt, off_mem_rsvmap); > grub_memcpy (tmp_fdt + off_dt_struct, > - fdt_ptr + grub_fdt_get_off_dt_struct (fdt), > - grub_fdt_get_size_dt_struct (fdt)); > + fdt_ptr + grub_fdt_get_off_dt_struct (fdt), > + grub_fdt_get_size_dt_struct (fdt)); > grub_fdt_set_off_dt_struct (fdt, off_dt_struct); > grub_memcpy (tmp_fdt + off_dt_strings, > - fdt_ptr + grub_fdt_get_off_dt_strings (fdt), > - grub_fdt_get_size_dt_strings (fdt)); > + fdt_ptr + grub_fdt_get_off_dt_strings (fdt), > + grub_fdt_get_size_dt_strings (fdt)); > grub_fdt_set_off_dt_strings (fdt, off_dt_strings); > > /* Copy reordered blocks back to fdt. */ > @@ -214,9 +230,12 @@ static grub_uint32_t *find_prop (void *fdt, unsigned int > nodeoffset, > grub_uint32_t *prop = (void *) ((grub_addr_t) fdt > + grub_fdt_get_off_dt_struct (fdt) > + nodeoffset); > + grub_uint32_t *end = (void *) struct_end(fdt); > grub_uint32_t nameoff; > + char *node_name; > > - do > + SKIP_NODE_NAME(node_name, prop, end); > + while (prop < end - 2) > { > if (grub_be_to_cpu32(*prop) == FDT_PROP) > { > @@ -224,15 +243,19 @@ static grub_uint32_t *find_prop (void *fdt, unsigned > int nodeoffset, > if ((nameoff + grub_strlen (name) < grub_fdt_get_size_dt_strings > (fdt)) > && !grub_strcmp (name, (char *) fdt + > grub_fdt_get_off_dt_strings (fdt) + nameoff)) > - return prop; > - prop += prop_entry_size(grub_be_to_cpu32(*prop + 1)) / sizeof > (*prop); > + { > + if (prop + prop_entry_size(grub_be_to_cpu32(*(prop + 1))) > + / sizeof (*prop) >= end) > + return NULL; > + return prop; > + } > + prop += prop_entry_size(grub_be_to_cpu32(*(prop + 1))) / sizeof > (*prop); > } > - else if (grub_be_to_cpu32(*prop) != FDT_NOP) > + else if (grub_be_to_cpu32(*prop) == FDT_NOP) > + prop++; > + else > return NULL; > - prop++; > - } while ((grub_addr_t) prop < ((grub_addr_t) fdt > - + grub_fdt_get_off_dt_struct (fdt) > - + grub_fdt_get_size_dt_struct (fdt))); > + } > return NULL; > } > > @@ -271,6 +294,9 @@ int grub_fdt_find_subnode (const void *fdt, unsigned int > parentoffset, > token = (void *) ((grub_addr_t) fdt + grub_fdt_get_off_dt_struct(fdt) > + parentoffset); > end = (void *) struct_end (fdt); > + if ((token >= end) || (grub_be_to_cpu32(*token) != FDT_BEGIN_NODE)) > + return -1; > + SKIP_NODE_NAME(node_name, token, end); > while (token < end) > { > switch (grub_be_to_cpu32(*token)) > @@ -280,19 +306,19 @@ int grub_fdt_find_subnode (const void *fdt, unsigned > int parentoffset, > if (node_name + grub_strlen (name) >= (char *) end) > return -1; > if (!grub_strcmp (node_name, name)) > - return (int) ((grub_addr_t) token > - + ALIGN_UP(grub_strlen (name) + 1, 4) > + return (int) ((grub_addr_t) token - (grub_addr_t) fdt > - grub_fdt_get_off_dt_struct (fdt)); > token = get_next_node (fdt, node_name); > if (!token) > return -1; > break; > - case FDT_END_NODE: > - return -1; > case FDT_PROP: > /* Skip property token and following data (len, nameoff and property > value). */ > - token += 3 + grub_be_to_cpu32 (*(token + 1)); > + if (token >= end - 1) > + return -1; > + token += prop_entry_size(grub_be_to_cpu32(*(token + 1))) > + / sizeof(*token); > break; > case FDT_NOP: > token++; > @@ -327,7 +353,10 @@ int grub_fdt_set_prop (void *fdt, unsigned int > nodeoffset, const char *name, > int prop_name_present = 0; > grub_uint32_t nameoff = 0; > > - if ((nodeoffset >= grub_fdt_get_size_dt_struct (fdt)) || (nodeoffset & > 0x3)) > + if ((nodeoffset >= grub_fdt_get_size_dt_struct (fdt)) || (nodeoffset & 0x3) > + || (grub_be_to_cpu32(*(grub_uint32_t *) ((grub_addr_t) fdt > + + grub_fdt_get_off_dt_struct (fdt) + nodeoffset)) > + != FDT_BEGIN_NODE)) > return -1; > prop = find_prop (fdt, nodeoffset, name); > if (prop) > @@ -339,7 +368,7 @@ int grub_fdt_set_prop (void *fdt, unsigned int > nodeoffset, const char *name, > prop_name_present = 1; > for (i = 0; i < prop_len / sizeof(grub_uint32_t); i++) > *(prop + 3 + i) = grub_cpu_to_be32 (FDT_NOP); > - if (len > prop_len) > + if (len > ALIGN_UP(prop_len, sizeof(grub_uint32_t))) > { > /* Length of new property value is greater than the space allocated > for the current value: a new entry needs to be created, so save > the > @@ -371,19 +400,24 @@ int grub_fdt_set_prop (void *fdt, unsigned int > nodeoffset, const char *name, > + grub_strlen (name) + 1); > } > if (!prop) { > - prop = (void *) ((grub_addr_t) fdt + grub_fdt_get_off_dt_struct (fdt) > - + nodeoffset); > - grub_memmove (prop + prop_entry_size(len), prop, > - grub_fdt_get_size_dt_struct (fdt) - nodeoffset); > + char *node_name = (char *) ((grub_addr_t) fdt > + + grub_fdt_get_off_dt_struct (fdt) + > nodeoffset > + + sizeof(grub_uint32_t)); > + > + prop = (void *) (node_name + ALIGN_UP(grub_strlen(node_name) + 1, 4)); > + grub_memmove (prop + prop_entry_size(len) / sizeof(*prop), prop, > + struct_end(fdt) - (grub_addr_t) prop); > + grub_fdt_set_size_dt_struct (fdt, grub_fdt_get_size_dt_struct (fdt) > + + prop_entry_size(len)); > *prop = grub_cpu_to_be32 (FDT_PROP); > - *(prop + 1) = grub_cpu_to_be32 (len); > *(prop + 2) = grub_cpu_to_be32 (nameoff); > + } > + *(prop + 1) = grub_cpu_to_be32 (len); > > - /* Insert padding bytes at the end of the value; if they are not needed, > - they will be overwritten by the follozing memcpy. */ > - *(prop + prop_entry_size(len) / sizeof(grub_uint32_t) - 1) = 0; > + /* Insert padding bytes at the end of the value; if they are not needed, > they > + will be overwritten by the following memcpy. */ > + *(prop + prop_entry_size(len) / sizeof(grub_uint32_t) - 1) = 0; > > - grub_memcpy (prop + 3, val, len); > - } > + grub_memcpy (prop + 3, val, len); > return 0; > } >
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel