Re: Various SIGFPEs on sh_entsize == 0

[Mark Wielaard <mjw at redhat dot com>]

On Tue, 2011-03-22 at 10:15 +0100, Petr Machata wrote:
> I hit these cases during my fuzzer runs.  I'm turning the patches in
> after yesterday's discussion on #elfutils.  My original inclination was
> that we don't have to support arbitrarily broken files, but it's true
> that the tools shouldn't misbehave either.  It's all on the branch
> pmachata/sh_entsize, and pasted here for easy review.

Yeah, I agree, if it is easy to detect, we should not crash.
You might want to consider to wrap the checks in unlikely () to indicate
they are exceptional cases.

> --- a/src/elflint.c
> +++ b/src/elflint.c
> @@ -1448,6 +1448,9 @@ check_rela (Ebl *ebl, GElf_Ehdr *ehdr, GElf_Shdr *shdr, int idx)
>    Elf_Data *symdata = elf_getdata (symscn, NULL);
>    enum load_state state = state_undecided;
>  
> +  if (shdr->sh_entsize == 0)
> +    return;
> +
>    for (size_t cnt = 0; cnt < shdr->sh_size / shdr->sh_entsize; ++cnt)
>      {
>        GElf_Rela rela_mem;

You probably want the same check in check_reloc_shdr and check_rel.

Maybe have a generic check in check_section for those shdr->sh_types
that are known to have fixed-size entries, with a shdr->sh_size > 0, but
sh_entsize == 0?

> diff --git a/src/readelf.c b/src/readelf.c
> index 30c2be0..17f657a 100644
> --- a/src/readelf.c
> +++ b/src/readelf.c
> @@ -1705,6 +1705,15 @@ static void
>  handle_relocs_rela (Ebl *ebl, GElf_Ehdr *ehdr, Elf_Scn *scn, GElf_Shdr *shdr)
>  {
>    int class = gelf_getclass (ebl->elf);
> +  if (shdr->sh_entsize == 0)
> +    {
> +      printf (gettext ("\
> +\nInvalid relocation section [%2zu] at offset %#0" PRIx64 ".\n"),
> +	      elf_ndxscn (scn),
> +	      shdr->sh_offset);
> +      return;
> +    }
> +
>    int nentries = shdr->sh_size / shdr->sh_entsize;

Same, here. Don't you also want that for handle_relocs_rel?

Thanks,

Mark