On architectures (like arm) that don't ALLOW_UNALIGNED there were a couple more issues with possible unbounded stack usage. The following 3 patches fix those:
libelf: Fix unbounded stack usage in elf_getarsym libelf: Fix possible unbounded stack usage in getphdr_wrlock. libelf: Fix possible unbounded stack usage in load_shdr_wrlock.
The number of entries in the index can be large, don't use alloca to read in temporary data, use malloc (which is freed after out).
Signed-off-by: Mark Wielaard mjw@redhat.com --- libelf/ChangeLog | 5 +++++ libelf/elf_getarsym.c | 10 +++++++++- 2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/libelf/ChangeLog b/libelf/ChangeLog index b749c08..4fd3f9f 100644 --- a/libelf/ChangeLog +++ b/libelf/ChangeLog @@ -1,3 +1,8 @@ +2015-05-31 Mark Wielaard mjw@redhat.com + + * elf_getarsym.c (elf_getarsym): Allocate temporary file_date with + malloc, not alloca also in !ALLOW_UNALIGNED case. + 2015-05-30 Mark Wielaard mjw@redhat.com
* gelf_xlate.c (elf_cvt_Byte): Only call memmove with non-zero size. diff --git a/libelf/elf_getarsym.c b/libelf/elf_getarsym.c index 4f2080a..8324244 100644 --- a/libelf/elf_getarsym.c +++ b/libelf/elf_getarsym.c @@ -255,7 +255,15 @@ elf_getarsym (elf, ptr) file_data = (void *) (elf->map_address + off); if (!ALLOW_UNALIGNED && ((uintptr_t) file_data & -(uintptr_t) n) != 0) - file_data = memcpy (alloca (sz), elf->map_address + off, sz); + { + temp_data = malloc (sz); + if (unlikely (temp_data == NULL)) + { + __libelf_seterrno (ELF_E_NOMEM); + goto out; + } + file_data = memcpy (temp_data, elf->map_address + off, sz); + } str_data = (char *) (elf->map_address + off + sz); }
When a copy needs to be made of the phdrs, allocate with malloc and free after conversion instead of calling alloca.
Signed-off-by: Mark Wielaard mjw@redhat.com --- libelf/ChangeLog | 5 +++++ libelf/elf32_getphdr.c | 18 ++++++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/libelf/ChangeLog b/libelf/ChangeLog index 4fd3f9f..65f9112 100644 --- a/libelf/ChangeLog +++ b/libelf/ChangeLog @@ -1,5 +1,10 @@ 2015-05-31 Mark Wielaard mjw@redhat.com
+ * elf32_getphdr.c (getphdr_wrlock): Allocate phdrs with malloc, not + alloca and free after conversion when a copy needs to be made. + +2015-05-31 Mark Wielaard mjw@redhat.com + * elf_getarsym.c (elf_getarsym): Allocate temporary file_date with malloc, not alloca also in !ALLOW_UNALIGNED case.
diff --git a/libelf/elf32_getphdr.c b/libelf/elf32_getphdr.c index 1b82a48..38e489d 100644 --- a/libelf/elf32_getphdr.c +++ b/libelf/elf32_getphdr.c @@ -141,13 +141,20 @@ __elfw2(LIBELFBITS,getphdr_wrlock) (elf) } else { - if (ALLOW_UNALIGNED - || ((uintptr_t) file_phdr - & (__alignof__ (ElfW2(LIBELFBITS,Phdr)) - 1)) == 0) + bool copy = ! (ALLOW_UNALIGNED + || ((uintptr_t) file_phdr + & (__alignof__ (ElfW2(LIBELFBITS,Phdr)) + - 1)) == 0); + if (! copy) notcvt = file_phdr; else { - notcvt = (ElfW2(LIBELFBITS,Phdr) *) alloca (size); + notcvt = (ElfW2(LIBELFBITS,Phdr) *) malloc (size); + if (unlikely (notcvt == NULL)) + { + __libelf_seterrno (ELF_E_NOMEM); + goto out; + } memcpy (notcvt, file_phdr, size); }
@@ -162,6 +169,9 @@ __elfw2(LIBELFBITS,getphdr_wrlock) (elf) CONVERT_TO (phdr[cnt].p_flags, notcvt[cnt].p_flags); CONVERT_TO (phdr[cnt].p_align, notcvt[cnt].p_align); } + + if (copy) + free (notcvt); } } }
When a copy needs to be made of the shdrs, allocate with malloc and free after conversion instead of calling alloca.
Signed-off-by: Mark Wielaard mjw@redhat.com --- libelf/ChangeLog | 5 +++++ libelf/elf32_getshdr.c | 18 ++++++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/libelf/ChangeLog b/libelf/ChangeLog index 65f9112..79308fe 100644 --- a/libelf/ChangeLog +++ b/libelf/ChangeLog @@ -1,5 +1,10 @@ 2015-05-31 Mark Wielaard mjw@redhat.com
+ * elf32_getshdr.c (load_shdr_wrlock): Allocate shdrs with malloc, + not alloca and free after conversion when a copy needs to be made. + +2015-05-31 Mark Wielaard mjw@redhat.com + * elf32_getphdr.c (getphdr_wrlock): Allocate phdrs with malloc, not alloca and free after conversion when a copy needs to be made.
diff --git a/libelf/elf32_getshdr.c b/libelf/elf32_getshdr.c index 7417047..ee1aed8 100644 --- a/libelf/elf32_getshdr.c +++ b/libelf/elf32_getshdr.c @@ -111,15 +111,22 @@ load_shdr_wrlock (Elf_Scn *scn) } else { - if (ALLOW_UNALIGNED - || ((uintptr_t) file_shdr - & (__alignof__ (ElfW2(LIBELFBITS,Shdr)) - 1)) == 0) + bool copy = ! (ALLOW_UNALIGNED + || ((uintptr_t) file_shdr + & (__alignof__ (ElfW2(LIBELFBITS,Shdr)) - 1)) + == 0); + if (! copy) notcvt = (ElfW2(LIBELFBITS,Shdr) *) ((char *) elf->map_address + elf->start_offset + ehdr->e_shoff); else { - notcvt = (ElfW2(LIBELFBITS,Shdr) *) alloca (size); + notcvt = (ElfW2(LIBELFBITS,Shdr) *) malloc (size); + if (unlikely (notcvt == NULL)) + { + __libelf_seterrno (ELF_E_NOMEM); + goto out; + } memcpy (notcvt, ((char *) elf->map_address + elf->start_offset + ehdr->e_shoff), size); @@ -153,6 +160,9 @@ load_shdr_wrlock (Elf_Scn *scn) elf->state.ELFW(elf,LIBELFBITS).scns.data[cnt].shndx_index = -1; } + + if (copy) + free (notcvt); } } else if (likely (elf->fildes != -1))
On Sun, 2015-05-31 at 21:05 +0200, Mark Wielaard wrote:
On architectures (like arm) that don't ALLOW_UNALIGNED there were a couple more issues with possible unbounded stack usage. The following 3 patches fix those:
libelf: Fix unbounded stack usage in elf_getarsym libelf: Fix possible unbounded stack usage in getphdr_wrlock. libelf: Fix possible unbounded stack usage in load_shdr_wrlock.
Pushed all 3 to master.
elfutils-devel@lists.fedorahosted.org