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

[Ido Yariv <ido@wizery.com>]

Hi Sjur,

On Fri, Dec 14, 2012 at 05:06:57PM +0100, Sjur Brændeland wrote:
> Refatcor rproc_elf_find_rsc_table and split out the scanning

Small typo there.

> for the section header named resource table. This is done to
> prepare for loading firmware once.
> 
> Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>

...

> +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.

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

Thanks,
Ido.