There were two issues with bogus sh_addralign values. First we would only check the individual Elf_Data d_align values were powerof2. But not the actual shdr addralign value. This would cause an issue if the shdr addralign was bigger than all of the individual d_align values. Then we could write out a bogus (! powerof2) shdr addralign value for the sections. Secondly when reading in the Elf_Data we would set the d_align value to the value of the shdr addralign value. But we would not check it was valid at all. In practice there are ELF files with incorrect sh_addralign values (they are a powerof2, but aren't aligned to the ELF image offset). We would try to fix that up in elf_update by adding extra padding. But this could bloat the ELF image a lot for large alignment values. So for too large alignments that are bigger than the offset in the ELF file clamp them to the offset value. This could lead us to reject to write out the data again when the offset was not a powerof2. But this will only happen for aligment values bigger than 64. Which are uncommon in practice. --- libelf/ChangeLog | 7 +++++++ libelf/elf32_updatenull.c | 5 +++++ libelf/elf_getdata.c | 14 ++++++++++++++ 3 files changed, 26 insertions(+) diff --git a/libelf/ChangeLog b/libelf/ChangeLog index a7983a0..0b9b478 100644 --- a/libelf/ChangeLog +++ b/libelf/ChangeLog @@ -1,5 +1,12 @@ 2015-05-12 Mark Wielaard + * elf32_updatenull.c (updatenull_wrlock): Check that sh_addralign + is a powerof2. + * elf_getdata.c (__libelf_set_rawdata_wrlock): Clamp large d_aligns + to the elf image offset. + +2015-05-12 Mark Wielaard + * elf32_newphdr.c (newphdr): Call __libelf_seterrno with ELF_E_INVALID_INDEX before failing. Check whether section zero shdr actually exists if we need to put extended phnum in section zero. diff --git a/libelf/elf32_updatenull.c b/libelf/elf32_updatenull.c index d873a30..a0de80e 100644 --- a/libelf/elf32_updatenull.c +++ b/libelf/elf32_updatenull.c @@ -202,6 +202,11 @@ __elfw2(LIBELFBITS,updatenull_wrlock) (Elf *elf, int *change_bop, size_t shnum) assert (shdr != NULL); ElfW2(LIBELFBITS,Word) sh_entsize = shdr->sh_entsize; ElfW2(LIBELFBITS,Word) sh_align = shdr->sh_addralign ?: 1; + if (unlikely (! powerof2 (sh_align))) + { + __libelf_seterrno (ELF_E_INVALID_ALIGN); + return -1; + } /* Set the sh_entsize value if we can reliably detect it. */ switch (shdr->sh_type) diff --git a/libelf/elf_getdata.c b/libelf/elf_getdata.c index e8f022f..8567da1 100644 --- a/libelf/elf_getdata.c +++ b/libelf/elf_getdata.c @@ -301,6 +301,20 @@ __libelf_set_rawdata_wrlock (Elf_Scn *scn) else scn->rawdata.d.d_type = shtype_map[LIBELF_EV_IDX][TYPEIDX (type)]; scn->rawdata.d.d_off = 0; + + /* Make sure the alignment makes sense. d_align should be aligned both + in the section (trivially true since d_off is zero) and in the file. + Unfortunately we cannot be too strict because there are ELF files + out there that fail this requirement. We will try to fix those up + in elf_update when writing out the image. But for very large + alignment values this can bloat the image considerably. So here + just check and clamp the alignment value to not be bigger than the + actual offset of the data in the file. Given that there is always + at least an ehdr this will only trigger for alignment values > 64 + which should be uncommon. */ + align = align ?: 1; + if (align > offset) + align = offset; scn->rawdata.d.d_align = align; if (elf->class == ELFCLASS32 || (offsetof (struct Elf, state.elf32.ehdr) -- 2.1.0