Re: [PATCH v9 10/27] xsplice: Add helper elf routines

["Jan Beulich" <JBeulich@suse.com>]

>>> On 26.04.16 at 12:05, <ross.lagerwall@citrix.com> wrote:
> On 04/25/2016 04:34 PM, Konrad Rzeszutek Wilk wrote:
> snip
>> +static int xsplice_header_check(const struct xsplice_elf *elf)
>> +{
>> +    const Elf_Ehdr *hdr = elf->hdr;
>> +
>> +    if ( sizeof(*elf->hdr) > elf->len )
>> +    {
>> +        dprintk(XENLOG_ERR, XSPLICE "%s: Section header is bigger than 
> payload!\n",
>> +                elf->name);
>> +        return -EINVAL;
>> +    }
>> +
>> +    if ( !IS_ELF(*hdr) )
>> +    {
>> +        dprintk(XENLOG_ERR, XSPLICE "%s: Not an ELF payload!\n", elf->name);
>> +        return -EINVAL;
>> +    }
>> +
>> +    if ( hdr->e_ident[EI_CLASS] != ELFCLASS64 ||
>> +         hdr->e_ident[EI_DATA] != ELFDATA2LSB ||
>> +         hdr->e_ident[EI_OSABI] != ELFOSABI_SYSV ||
>> +         hdr->e_type != ET_REL ||
>> +         hdr->e_phnum != 0 )
>> +    {
>> +        dprintk(XENLOG_ERR, XSPLICE "%s: Invalid ELF payload!\n", elf->name);
>> +        return -EOPNOTSUPP;
>> +    }
>> +
>> +    if ( elf->hdr->e_shstrndx == SHN_UNDEF )
>> +    {
>> +        dprintk(XENLOG_ERR, XSPLICE "%s: Section name idx is 
> undefined!?\n",
>> +                elf->name);
>> +        return -EINVAL;
>> +    }
>> +
>> +    /* Check that section name index is within the sections. */
>> +    if ( elf->hdr->e_shstrndx >= elf->hdr->e_shnum )
>> +    {
>> +        dprintk(XENLOG_ERR, XSPLICE "%s: Section name idx (%u) is past end 
> of sections (%u)!\n",
>> +                elf->name, elf->hdr->e_shstrndx, elf->hdr->e_shnum);
>> +        return -EINVAL;
>> +    }
>> +
>> +    if ( elf->hdr->e_shnum > 64 )
>> +    {
>> +        dprintk(XENLOG_ERR, XSPLICE "%s: Too many (%u) sections!\n",
>> +                elf->name, elf->hdr->e_shnum);
>> +        return -EOPNOTSUPP;
>> +    }
> 
> If I recall correctly, Andrew asked you to add this check. Due to 
> compiling with -ffunction-sections -fdata-sections, the build tool can 
> quite easily exceed this limit. IMO the check doesn't serve any useful 
> purpose and should be removed.

Well, it certainly serves the purpose of subsequent things not taking
overly long, but I'd be fine with the limit bumped. And the check can't
go away altogether anyway - at the very least you need to check
against SHN_LORESERVE.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel