RE: [RFCv2 08/11] remoteproc: Refactor function rproc_elf_find_rsc_table

Sjur BRENDELAND
2013-01-15T15:11:58Z

Hi Ido,

> > +static struct elf32_shdr *
> > +find_rsc_shdr(struct device *dev, struct elf32_hdr *ehdr)
> > {
> > - struct elf32_hdr *ehdr;
> > struct elf32_shdr *shdr;
> > + int i;
> > const char *name_table;
> > - struct device *dev = &rproc->dev;
> > struct resource_table *table = NULL;
> > - int i;
> > - const u8 *elf_data = fw->data;
> > + const u8 *elf_data = (void *)ehdr;
> >
> > - ehdr = (struct elf32_hdr *)elf_data;
> > + /* look for the resource table and handle it */
> > shdr = (struct elf32_shdr *)(elf_data + ehdr->e_shoff);
> > name_table = elf_data + shdr[ehdr->e_shstrndx].sh_offset;
> >
> > - /* look for the resource table and handle it */
> > for (i = 0; i < ehdr->e_shnum; i++, shdr++) {
> > int size = shdr->sh_size;
> > int offset = shdr->sh_offset;
> > @@ -249,12 +230,6 @@ rproc_elf_find_rsc_table(struct rproc *rproc, const
> struct firmware *fw,
> >
> > table = (struct resource_table *)(elf_data + offset);
> >
> > - /* make sure we have the entire table */
> > - if (offset + size > fw->size) {
> > - dev_err(dev, "resource table truncated\n");
> > - return NULL;
> > - }
> > -
>
> This should probably be kept in the internal function, since it
> dereferences the table as well. Moreover, this function will also be
> called from other function locations.

OK, I can do that.

>
> It might also be a good idea to verify the offset as well, not just the
> size.

I'm not sure what you have in mind here. What sort of checks would you do on offset?

Regards,
Sjur