GCC 5.1 includes -fsanitize=nonnull-attribute in -fsanitize=undefined. This showed a couple of places where we were passing a NULL argument to functions not expecting them. Even though these functions might not actually use the argument (if for example the size argument of a buffer is zero), gcc might use the information to assume the passed in variables are always nonnull, which might cause unexpected optimizations.
libdwfl: Check file_name is not NULL before calling canonicalize_file_name. libelf: Don't call mempcpy with possible NULL d_buf. libelf: Don't call memmove with possible NULL buffer. elfcmp: Don't call memcmp with possible NULL d_buf.
canonicalize_file_name is marked as taking a nonnull argument.
Signed-off-by: Mark Wielaard mjw@redhat.com --- libdwfl/ChangeLog | 5 +++++ libdwfl/find-debuginfo.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog index dec3405..ffb34c0 100644 --- a/libdwfl/ChangeLog +++ b/libdwfl/ChangeLog @@ -1,3 +1,8 @@ +2015-05-30 Mark Wielaard mjw@redhat.com + + * find-debuginfo.c (dwfl_standard_find_debuginfo): Check file_name is + not NULL before calling canonicalize_file_name. + 2015-05-24 Mark Wielaard mjw@redhat.com
* derelocate.c (check_module): Check mod is not NULL. diff --git a/libdwfl/find-debuginfo.c b/libdwfl/find-debuginfo.c index ac9a5e5..9b911c1 100644 --- a/libdwfl/find-debuginfo.c +++ b/libdwfl/find-debuginfo.c @@ -357,7 +357,7 @@ dwfl_standard_find_debuginfo (Dwfl_Module *mod, debuglink_file, debuglink_crc, debuginfo_file_name);
- if (fd < 0 && errno == 0) + if (fd < 0 && errno == 0 && file_name != NULL) { /* If FILE_NAME is a symlink, the debug file might be associated with the symlink target name instead. */
When d_size is zero d_buf might be NULL. last_position doesn't need to be updated in that case.
Signed-off-by: Mark Wielaard mjw@redhat.com --- libelf/ChangeLog | 5 +++++ libelf/elf32_updatefile.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/libelf/ChangeLog b/libelf/ChangeLog index 5f66135..b8c20cc 100644 --- a/libelf/ChangeLog +++ b/libelf/ChangeLog @@ -1,3 +1,8 @@ +2015-05-30 Mark Wielaard mjw@redhat.com + + * elf32_updatefile.c (updatemmap): Only call mempcpy and update + last_position when d_size is non-zero. + 2015-05-17 Mark Wielaard mjw@redhat.com
* elf32_updatefile.c (updatefile): Allocate shdr_data and scns diff --git a/libelf/elf32_updatefile.c b/libelf/elf32_updatefile.c index e90ad47..913c3e4 100644 --- a/libelf/elf32_updatefile.c +++ b/libelf/elf32_updatefile.c @@ -367,7 +367,7 @@ __elfw2(LIBELFBITS,updatemmap) (Elf *elf, int change_bo, size_t shnum)
last_position += dl->data.d.d_size; } - else + else if (dl->data.d.d_size != 0) last_position = mempcpy (last_position, dl->data.d.d_buf, dl->data.d.d_size);
When size is zero the buffer src and dest buffers might be NULL.
Signed-off-by: Mark Wielaard mjw@redhat.com --- libelf/ChangeLog | 4 ++++ libelf/gelf_xlate.c | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/libelf/ChangeLog b/libelf/ChangeLog index b8c20cc..b749c08 100644 --- a/libelf/ChangeLog +++ b/libelf/ChangeLog @@ -1,5 +1,9 @@ 2015-05-30 Mark Wielaard mjw@redhat.com
+ * gelf_xlate.c (elf_cvt_Byte): Only call memmove with non-zero size. + +2015-05-30 Mark Wielaard mjw@redhat.com + * elf32_updatefile.c (updatemmap): Only call mempcpy and update last_position when d_size is non-zero.
diff --git a/libelf/gelf_xlate.c b/libelf/gelf_xlate.c index c417051..c5805e7 100644 --- a/libelf/gelf_xlate.c +++ b/libelf/gelf_xlate.c @@ -1,5 +1,5 @@ /* Transformation functions for ELF data types. - Copyright (C) 1998,1999,2000,2002,2004,2005,2006,2007 Red Hat, Inc. + Copyright (C) 1998,1999,2000,2002,2004,2005,2006,2007,2015 Red Hat, Inc. This file is part of elfutils. Written by Ulrich Drepper drepper@redhat.com, 1998.
@@ -52,7 +52,8 @@ static void (elf_cvt_Byte) (void *dest, const void *src, size_t n, int encode __attribute__ ((unused))) { - memmove (dest, src, n); + if (n != 0) + memmove (dest, src, n); }
When d_size is zero d_buf might be NULL.
Signed-off-by: Mark Wielaard mjw@redhat.com --- src/ChangeLog | 4 ++++ src/elfcmp.c | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/ChangeLog b/src/ChangeLog index 3161aa8..59d87c8 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,7 @@ +2015-05-30 Mark Wielaard mjw@redhat.com + + * elfcmp.c (main): Only call memcmp when d_size != 0. + 2015-05-23 Mark Wielaard mjw@redhat.com
* Makefile.am: Define ldgeneric, readelf, nm, size, strip, elflint, diff --git a/src/elfcmp.c b/src/elfcmp.c index f8a4572..0250fbe 100644 --- a/src/elfcmp.c +++ b/src/elfcmp.c @@ -1,5 +1,5 @@ /* Compare relevant content of two ELF files. - Copyright (C) 2005-2012, 2014 Red Hat, Inc. + Copyright (C) 2005-2012, 2014, 2015 Red Hat, Inc. This file is part of elfutils. Written by Ulrich Drepper drepper@redhat.com, 2005.
@@ -511,6 +511,7 @@ cannot read note section [%zu] '%s' in '%s': %s"),
if (unlikely (data1->d_size != data2->d_size || (shdr1->sh_type != SHT_NOBITS + && data1->d_size != 0 && memcmp (data1->d_buf, data2->d_buf, data1->d_size) != 0))) {
On Sun, 2015-05-31 at 17:51 +0200, Mark Wielaard wrote:
GCC 5.1 includes -fsanitize=nonnull-attribute in -fsanitize=undefined. This showed a couple of places where we were passing a NULL argument to functions not expecting them. Even though these functions might not actually use the argument (if for example the size argument of a buffer is zero), gcc might use the information to assume the passed in variables are always nonnull, which might cause unexpected optimizations.
libdwfl: Check file_name is not NULL before calling canonicalize_file_name. libelf: Don't call mempcpy with possible NULL d_buf. libelf: Don't call memmove with possible NULL buffer. elfcmp: Don't call memcmp with possible NULL d_buf.
Pushed all 4 commits to master.
elfutils-devel@lists.fedorahosted.org