In message <4be78e06.6080...@ozlabs.org> you wrote:
> 
> ppc64's fs2dt used to use a fixed-size array into which the device tree
> was parsed.  There was no bounds checking, so with a large device tree other
> heap data ended up getting stomped -- SIGSEGV time.
> 
> This patch adds a function, 'dt_reserve', to check whether there's enough spa
ce
> left prior to writing data to the array.  If not, the array is realloced.
> 
> Signed-off-by: Matt Evans <m...@ozlabs.org>

FWIW...

Ack-by: Michael Neuling <mi...@neuling.org>

(also added linuxppc-...@ozlabs.org to CC list)

> ---
>  kexec/arch/ppc64/fs2dt.c |   62 +++++++++++++++++++++++++++++++++++++++-----
-
>  1 files changed, 53 insertions(+), 9 deletions(-)
> 
> diff --git a/kexec/arch/ppc64/fs2dt.c b/kexec/arch/ppc64/fs2dt.c
> index f168cbc..762bf04 100644
> --- a/kexec/arch/ppc64/fs2dt.c
> +++ b/kexec/arch/ppc64/fs2dt.c
> @@ -35,13 +35,14 @@
>  
>  #define MAXPATH 1024         /* max path name length */
>  #define NAMESPACE 16384              /* max bytes for property names */
> -#define TREEWORDS 65536              /* max 32 bit words for property values
 */
> +#define INIT_TREE_WORDS 65536        /* Initial num words for prop values */
>  #define MEMRESERVE 256               /* max number of reserved memory blocks
 */
>  #define MAX_MEMORY_RANGES 1024
>  
>  static char pathname[MAXPATH], *pathstart;
>  static char propnames[NAMESPACE] = { 0 };
> -static unsigned dtstruct[TREEWORDS] __attribute__ ((aligned (8))), *dt;
> +static unsigned *dt_base, *dt;
> +static unsigned int dt_cur_size;
>  static unsigned long long mem_rsrv[2*MEMRESERVE] = { 0, 0 };
>  
>  static int crash_param = 0;
> @@ -50,6 +51,28 @@ extern mem_rgns_t usablemem_rgns;
>  static struct bootblock bb[1];
>  extern int my_debug;
>  
> +/* Before we add something to the dt, reserve N words using this.
> + * If there isn't enough room, it's realloced -- and you don't overflow and
> + * splat bits of your heap. 
> + */
> +void dt_reserve(unsigned **dt_ptr, unsigned words)
> +{
> +     if (((*dt_ptr - dt_base) + words) >= dt_cur_size) {
> +             int offset;
> +             unsigned int new_size = dt_cur_size + INIT_TREE_WORDS;
> +             unsigned *new_dt = realloc(dt_base, new_size*4);
> +
> +             if (!new_dt)
> +                     die("unrecoverable error: Can't realloc %d bytes for "
> +                         "device tree\n", new_size*4);
> +             offset = *dt_ptr - dt_base;
> +             dt_base = new_dt;
> +             dt_cur_size = new_size;
> +             *dt_ptr = dt_base + offset;
> +             memset(*dt_ptr, 0, (new_size - offset)*4);
> +     }
> +}
> +
>  void reserve(unsigned long long where, unsigned long long length)
>  {
>       size_t offset;
> @@ -181,6 +204,7 @@ static void add_dyn_reconf_usable_mem_property(int fd)
>       /*
>        * Add linux,drconf-usable-memory property.
>        */
> +     dt_reserve(&dt, 4+((rlen + 3)/4));
>       *dt++ = 3;
>       *dt++ = rlen;
>       *dt++ = propnum("linux,drconf-usable-memory");
> @@ -253,6 +277,7 @@ static void add_usable_mem_property(int fd, size_t len)
>       /*
>        * No add linux,usable-memory property.
>        */
> +     dt_reserve(&dt, 4+((rlen + 3)/4));
>       *dt++ = 3;
>       *dt++ = rlen;
>       *dt++ = propnum("linux,usable-memory");
> @@ -317,6 +342,7 @@ static void putprops(char *fn, struct dirent **nlist, int
 numlist)
>  
>               len = statbuf.st_size;
>  
> +             dt_reserve(&dt, 4+((len + 3)/4));
>               *dt++ = 3;
>               *dt++ = len;
>               *dt++ = propnum(fn);
> @@ -388,13 +414,17 @@ static void putnode(void)
>       struct dirent **namelist;
>       int numlist, i;
>       struct stat statbuf;
> +     int plen;
>  
> +     plen = *pathstart ? strlen(pathstart) : 1;
> +     /* Reserve space for string packed to words; e.g. string length 10 
> +      * occupies 3 words, length 12 occupies 4 (for terminating \0s).  
> +      * So round up & include the \0:
> +      */
> +     dt_reserve(&dt, 1+((plen + 4)/4));
>       *dt++ = 1;
>       strcpy((void *)dt, *pathstart ? pathstart : "/");
> -     while(*dt)
> -             dt++;
> -     if (dt[-1] & 0xff)
> -             dt++;
> +     dt += ((plen + 4)/4);
>  
>       numlist = scandir(pathname, &namelist, 0, comparefunc);
>       if (numlist < 0)
> @@ -415,6 +445,8 @@ static void putnode(void)
>       if (initrd_base && !strcmp(basename,"/chosen/")) {
>               int len = 8;
>               unsigned long long initrd_end;
> +
> +             dt_reserve(&dt, 12); /* both props, of 6 words ea. */
>               *dt++ = 3;
>               *dt++ = len;
>               *dt++ = propnum("linux,initrd-start");
> @@ -487,6 +519,7 @@ static void putnode(void)
>               cmd_len = cmd_len + 1;
>  
>               /* add new bootargs */
> +             dt_reserve(&dt, 4+((cmd_len+3)/4));
>               *dt++ = 3;
>               *dt++ = cmd_len;
>               *dt++ = propnum("bootargs");
> @@ -571,6 +604,7 @@ no_debug:
>                       putnode();
>       }
>  
> +     dt_reserve(&dt, 1);
>       *dt++ = 2;
>       dn[-1] = '\0';
>       free(namelist);
> @@ -588,12 +622,21 @@ int create_flatten_tree(char **bufp, off_t *sizep, char
 *cmdline)
>       strcpy(pathname, "/proc/device-tree/");
>  
>       pathstart = pathname + strlen(pathname);
> -     dt = dtstruct;
> +
> +     dt_cur_size = INIT_TREE_WORDS;
> +     dt_base = malloc(dt_cur_size*4);
> +     if (!dt_base) {
> +             die("Can't malloc %d bytes for dt struct!\n", dt_cur_size*4);
> +     }
> +     memset(dt_base, 0, dt_cur_size*4);
> +
> +     dt = dt_base;
>  
>       if (cmdline)
>               strcpy(local_cmdline, cmdline);
>  
>       putnode();
> +     dt_reserve(&dt, 1);
>       *dt++ = 9;
>  
>       len = sizeof(bb[0]);
> @@ -608,7 +651,7 @@ int create_flatten_tree(char **bufp, off_t *sizep, char *
cmdline)
>  
>       bb->off_dt_struct = bb->off_mem_rsvmap + len;
>  
> -     len = dt - dtstruct;
> +     len = dt - dt_base;
>       len *= sizeof(unsigned);
>       bb->off_dt_strings = bb->off_dt_struct + len;
>  
> @@ -628,10 +671,11 @@ int create_flatten_tree(char **bufp, off_t *sizep, char
 *cmdline)
>       tlen = bb->off_mem_rsvmap;
>       memcpy(buf+tlen, mem_rsrv, bb->off_dt_struct - bb->off_mem_rsvmap);
>       tlen = tlen + (bb->off_dt_struct - bb->off_mem_rsvmap);
> -     memcpy(buf+tlen, dtstruct,  bb->off_dt_strings - bb->off_dt_struct);
> +     memcpy(buf+tlen, dt_base,  bb->off_dt_strings - bb->off_dt_struct);
>       tlen = tlen +  (bb->off_dt_strings - bb->off_dt_struct);
>       memcpy(buf+tlen, propnames,  bb->totalsize - bb->off_dt_strings);
>       tlen = tlen + bb->totalsize - bb->off_dt_strings;
>       *sizep = tlen;
> +     free(dt_base);
>       return 0;
>  }
> -- 
> 1.6.3.3
> 
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to