Re: [PATCH] libelf/elf_end.c: check data_list.data.d.d_buf before free it
On 08/17/2018 03:25 AM, Mark Wielaard wrote: Hi, On Thu, Aug 16, 2018 at 10:34:23AM +0800, Robert Yang wrote: The one which actually saves the data is data_list.data.d.d_buf, so check it before free rawdata_base. This can fix a segmentation fault when prelink libqb_1.0.3: prelink: /usr/lib/libqb.so.0.18.2: Symbol section index outside of section numbers The segmentation fault happens when prelink call elf_end(). Could you run your reproducer under valgrind and show what it says before your patch? And/Or post the file (libqb) to replicate the reproducer somewhere to see exactly what goes wrong? I don't fully understand what is going wrong. Is the section data pointing to the file data or something created by elf_newdata? Thanks for the reply, I found this problem in a cross build environment, but we are using elfutils as native tool (directly running on host), its version is 0.172, srcrev=01e87ab4c5a6a249c04e22a97a4221d3. $ VALGRIND_LIB=/path/to/usr/lib/valgrind valgrind prelink --root /path/to/core-image-minimal/1.0-r0/rootfs -amR -N -c /etc/prelink.conf --dynamic-linker /lib/ld-linux-x86-64.so.2 -v Here are the problems related to elfutils: prelink: /usr/lib/libqb.so.0.19.0: Symbol section index outside of section numbers ==25330== Invalid free() / delete / delete[] / realloc() ==25330==at 0x4C3026B: free (in /path/to/recipe-sysroot-native/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==25330==by 0x50991F5: elf_end (elf_end.c:171) ==25330==by 0x41E916: ??? (in /path/to/recipe-sysroot-native/usr/sbin/prelink) ==25330==by 0x422233: ??? (in /path/to/recipe-sysroot-native/usr/sbin/prelink) ==25330==by 0x408BE1: ??? (in /path/to/recipe-sysroot-native/usr/sbin/prelink) ==25330==by 0x409015: ??? (in /path/to/recipe-sysroot-native/usr/sbin/prelink) ==25330==by 0x4038FB: ??? (in /path/to/recipe-sysroot-native/usr/sbin/prelink) ==25330==by 0x52CF04A: (below main) (in /buildarea1/lyang1/test_up/tmp/sysroots-uninative/x86_64-linux/lib/libc-2.27.so) ==25330== Address 0x9305300 is 0 bytes inside a block of size 20 free'd ==25330==at 0x4C3026B: free (in /path/to/recipe-sysroot-native/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==25330==by 0x41E8B7: ??? (in /path/to/recipe-sysroot-native/usr/sbin/prelink) ==25330==by 0x422233: ??? (in /path/to/recipe-sysroot-native/usr/sbin/prelink) ==25330==by 0x408BE1: ??? (in /path/to/recipe-sysroot-native/usr/sbin/prelink) ==25330==by 0x409015: ??? (in /path/to/recipe-sysroot-native/usr/sbin/prelink) ==25330==by 0x4038FB: ??? (in /path/to/recipe-sysroot-native/usr/sbin/prelink) ==25330==by 0x52CF04A: (below main) (in /buildarea1/lyang1/test_up/tmp/sysroots-uninative/x86_64-linux/lib/libc-2.27.so) ==25330== Block was alloc'd at ==25330==at 0x4C2F03F: malloc (in /path/to/recipe-sysroot-native/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==25330==by 0x509DE8E: __libelf_set_rawdata_wrlock (elf_getdata.c:329) ==25330==by 0x509E20E: __elf_getdata_rdlock (elf_getdata.c:532) ==25330==by 0x509E24D: elf_getdata (elf_getdata.c:559) ==25330==by 0x420A70: ??? (in /path/to/recipe-sysroot-native/usr/sbin/prelink) ==25330==by 0x413860: ??? (in /path/to/recipe-sysroot-native/usr/sbin/prelink) ==25330==by 0x408D64: ??? (in /path/to/recipe-sysroot-native/usr/sbin/prelink) ==25330==by 0x409015: ??? (in /path/to/recipe-sysroot-native/usr/sbin/prelink) ==25330==by 0x4038FB: ??? (in /path/to/recipe-sysroot-native/usr/sbin/prelink) ==25330==by 0x52CF04A: (below main) (in /buildarea1/lyang1/test_up/tmp/sysroots-uninative/x86_64-linux/lib/libc-2.27.so) // Robert Thanks, Mark
Re: [PATCH] libelf/elf_end.c: check data_list.data.d.d_buf before free it
[rereading, two prelink commands got missing and diff ones were
garbled in the sequence below, losing consequentiality, sorry]
On 16/08/18 22:15 +0200, Jan Pokorný wrote:
> Out of curiousity, tried this on my Fedora machine without any
> success to reproduce:
>
> # dnf install -y libqb
> https://kojipkgs.fedoraproject.org//packages/prelink/0.5.0/1.fc19/x86_64/prelink-0.5.0-1.fc19.x86_64.rpm
> # chmod -x /etc/cron.daily/prelink
> # cp /usr/lib64/libqb.so.0.19.0{,.bck}
# prelink /usr/lib64/libqb.so.0.19.0
# diff /usr/lib64/libqb.so.0.19.0{,.bck} >/dev/null && echo same || echo not
> > not
> # dnf downgrade -y
> https://kojipkgs.fedoraproject.org//packages/libqb/1.0.2/1.fc26/x86_64/libqb-1.0.2-1.fc26.x86_64.rpm
> # cp /usr/lib64/libqb.so.0.18.2{,.bck}
# prelink /usr/lib64/libqb.so.0.18.2
# diff /usr/lib64/libqb.so.0.18.2{,.bck} >/dev/null && echo same || echo not
> > not
--
Nazdar,
Jan (Poki)
pgpA2zueENWt5.pgp
Description: PGP signature
Re: [PATCH] libelf/elf_end.c: check data_list.data.d.d_buf before free it
Hi Robert, [I don't have very good internet connectivity so cannot easily get all the bits and sources to replicate/inspect. So apologies if I am misinterpreting something.] On Fri, Aug 17, 2018 at 04:25:07PM +0800, Robert Yang wrote: > On 08/17/2018 03:25 AM, Mark Wielaard wrote: > > On Thu, Aug 16, 2018 at 10:34:23AM +0800, Robert Yang wrote: > > > The one which actually saves the data is data_list.data.d.d_buf, so check > > > it > > > before free rawdata_base. > > > > > > This can fix a segmentation fault when prelink libqb_1.0.3: > > > prelink: /usr/lib/libqb.so.0.18.2: Symbol section index outside of > > > section numbers > > > > > > The segmentation fault happens when prelink call elf_end(). > > > > Could you run your reproducer under valgrind and show what it > > says before your patch? And/Or post the file (libqb) to replicate > > the reproducer somewhere to see exactly what goes wrong? > > > > I don't fully understand what is going wrong. Is the section data > > pointing to the file data or something created by elf_newdata? > > Thanks for the reply, I found this problem in a cross build environment, > but we are using elfutils as native tool (directly running on host), its > version is 0.172, srcrev=01e87ab4c5a6a249c04e22a97a4221d3. > > $ VALGRIND_LIB=/path/to/usr/lib/valgrind valgrind prelink --root > /path/to/core-image-minimal/1.0-r0/rootfs -amR -N -c /etc/prelink.conf > --dynamic-linker /lib/ld-linux-x86-64.so.2 -v > > Here are the problems related to elfutils: > > prelink: /usr/lib/libqb.so.0.19.0: Symbol section index outside of section > numbers > ==25330== Invalid free() / delete / delete[] / realloc() > ==25330==at 0x4C3026B: free (in > /path/to/recipe-sysroot-native/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > ==25330==by 0x50991F5: elf_end (elf_end.c:171) > ==25330==by 0x41E916: ??? (in > /path/to/recipe-sysroot-native/usr/sbin/prelink) > ==25330==by 0x422233: ??? (in > /path/to/recipe-sysroot-native/usr/sbin/prelink) > ==25330==by 0x408BE1: ??? (in > /path/to/recipe-sysroot-native/usr/sbin/prelink) > ==25330==by 0x409015: ??? (in > /path/to/recipe-sysroot-native/usr/sbin/prelink) > ==25330==by 0x4038FB: ??? (in > /path/to/recipe-sysroot-native/usr/sbin/prelink) > ==25330==by 0x52CF04A: (below main) (in > /buildarea1/lyang1/test_up/tmp/sysroots-uninative/x86_64-linux/lib/libc-2.27.so) > ==25330== Address 0x9305300 is 0 bytes inside a block of size 20 free'd > ==25330==at 0x4C3026B: free (in > /path/to/recipe-sysroot-native/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > ==25330==by 0x41E8B7: ??? (in > /path/to/recipe-sysroot-native/usr/sbin/prelink) > ==25330==by 0x422233: ??? (in > /path/to/recipe-sysroot-native/usr/sbin/prelink) > ==25330==by 0x408BE1: ??? (in > /path/to/recipe-sysroot-native/usr/sbin/prelink) > ==25330==by 0x409015: ??? (in > /path/to/recipe-sysroot-native/usr/sbin/prelink) > ==25330==by 0x4038FB: ??? (in > /path/to/recipe-sysroot-native/usr/sbin/prelink) > ==25330==by 0x52CF04A: (below main) (in > /buildarea1/lyang1/test_up/tmp/sysroots-uninative/x86_64-linux/lib/libc-2.27.so) > ==25330== Block was alloc'd at > ==25330==at 0x4C2F03F: malloc (in > /path/to/recipe-sysroot-native/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > ==25330==by 0x509DE8E: __libelf_set_rawdata_wrlock (elf_getdata.c:329) > ==25330==by 0x509E20E: __elf_getdata_rdlock (elf_getdata.c:532) > ==25330==by 0x509E24D: elf_getdata (elf_getdata.c:559) > ==25330==by 0x420A70: ??? (in > /path/to/recipe-sysroot-native/usr/sbin/prelink) > ==25330==by 0x413860: ??? (in > /path/to/recipe-sysroot-native/usr/sbin/prelink) > ==25330==by 0x408D64: ??? (in > /path/to/recipe-sysroot-native/usr/sbin/prelink) > ==25330==by 0x409015: ??? (in > /path/to/recipe-sysroot-native/usr/sbin/prelink) > ==25330==by 0x4038FB: ??? (in > /path/to/recipe-sysroot-native/usr/sbin/prelink) > ==25330==by 0x52CF04A: (below main) (in > /buildarea1/lyang1/test_up/tmp/sysroots-uninative/x86_64-linux/lib/libc-2.27.so) Thanks, that does suggest to me there is a bug in prelink. Although it isn't completely clear. If you could run the same with prelink debuginfo so we can see the source lines that would be great. The reason I think this is a prelink issues is because it looks like it is calling elf_getdata () to get the data, and then frees the buffer. The idea is that you only own the data of an elf section if you created it yourself with elf_newdata (). Otherwise, as seems to have happened here, libelf owns the data buffer and has to free it. I think that prelink is preparing the ELF file, but then half way through encounters an error. It then frees all the ELF section data it believed it created itself. But because of the error it probably didn't (yet) do that. And so frees some data that it got directly from elf_getdata () and didn't create itself. Then it calls elf_end () and lib
[Bug libdw/23541] heap-buffer-overflow in /elfutils/libdw/dwarf_getaranges.c:156
https://sourceware.org/bugzilla/show_bug.cgi?id=23541
Mark Wielaard changed:
What|Removed |Added
CC||mark at klomp dot org
--- Comment #1 from Mark Wielaard ---
I couldn't replicate with the given reproducer.
But it is pretty clear that we forget to check there is enough data available
when reading the aranges header in dwarf_getaranges.c. In particular we forget
to check we can actually read the address and segment size bytes.
diff --git a/libdw/dwarf_getaranges.c b/libdw/dwarf_getaranges.c
index bff9c860..de5b81ba 100644
--- a/libdw/dwarf_getaranges.c
+++ b/libdw/dwarf_getaranges.c
@@ -148,6 +148,10 @@ dwarf_getaranges (Dwarf *dbg, Dwarf_Aranges **aranges,
size_t *naranges)
length_bytes, &offset, IDX_debug_info, 4))
goto fail;
+ /* Next up two bytes for address and segment size. */
+ if (readp + 2 > readendp)
+ goto invalid;
+
unsigned int address_size = *readp++;
if (unlikely (address_size != 4 && address_size != 8))
goto invalid;
While checking similar code in readelf.c I found we have a similar check
missing, but this time just for the segment size:
diff --git a/src/readelf.c b/src/readelf.c
index 7b5707f8..7b488ac5 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -5447,6 +5447,8 @@ print_debug_aranges_section (Dwfl_Module *dwflmod
__attribute__ ((unused)),
goto next_table;
}
+ if (readp + 1 > readendp)
+ goto invalid_data;
unsigned int segment_size = *readp++;
printf (gettext (" Segment size: %6" PRIu64 "\n\n"),
(uint64_t) segment_size);
It looks like all other checks are in place, but this code could probably
benefit from some extra fuzzing.
--
You are receiving this mail because:
You are on the CC list for the bug.
[Bug general/23542] heap-buffer-overflow in /elfutils/src/elflint.c:2055 check_sysv_hash
https://sourceware.org/bugzilla/show_bug.cgi?id=23542
Mark Wielaard changed:
What|Removed |Added
CC||mark at klomp dot org
--- Comment #1 from Mark Wielaard ---
Replicated under valgrind:
==12265== Conditional jump or move depends on uninitialised value(s)
==12265==at 0xE9: check_sysv_hash (elflint.c:2056)
==12265==by 0xE9: check_hash.isra.14 (elflint.c:2356)
==12265==by 0x117B80: check_sections (elflint.c:4162)
==12265==by 0x119364: process_elf_file (elflint.c:4740)
==12265==by 0x119364: process_file (elflint.c:242)
==12265==by 0x10C57C: main (elflint.c:175)
The issue is that the sanity check at the start of the function overflows
because it does 32bit unsigned arithmetic. Changing it to do unsigned long long
arithmetic makes the check catch the issue:
diff --git a/src/elflint.c b/src/elflint.c
index eec799b2..9d49c47f 100644
--- a/src/elflint.c
+++ b/src/elflint.c
@@ -2023,7 +2023,7 @@ check_sysv_hash (Ebl *ebl, GElf_Shdr *shdr, Elf_Data
*data, int idx,
Elf32_Word nbucket = ((Elf32_Word *) data->d_buf)[0];
Elf32_Word nchain = ((Elf32_Word *) data->d_buf)[1];
- if (shdr->sh_size < (2 + nbucket + nchain) * sizeof (Elf32_Word))
+ if (shdr->sh_size < (2ULL + nbucket + nchain) * sizeof (Elf32_Word))
{
ERROR (gettext ("\
section [%2d] '%s': hash table section is too small (is %ld, expected
%ld)\n"),
--
You are receiving this mail because:
You are on the CC list for the bug.
