>From 1be76fe8259d0a7dacf1b357a8b18738234bab1b Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Tue, 20 Jan 2015 21:55:55 +0100 Subject: [PATCH] libelf: Fix elf_newdata when raw ELF file/image data is available. When ELF data for a section has been read by elf_rawdata, data_read and rawdata_base are set, but data_list_rear will not be set until the data will be converted (by elf_getdata). elf_newdata would overwrite the existing data in that case. Both elf_getdata and elf_update rely on the fact that when data_list_rear is set they don't have to look at the raw data anymore. So make sure we update the data list properly before adding any new data and raw data is available in elf_newdata. Add newdata test that calls elf_newdata before and after elf_rawdata and elf_getdata and checks the new size and contents of the section. Signed-off-by: Mark Wielaard --- libelf/ChangeLog | 9 ++ libelf/elf_getdata.c | 76 +++++----- libelf/elf_newdata.c | 39 ++++- libelf/libelfP.h | 6 + tests/ChangeLog | 7 + tests/Makefile.am | 5 +- tests/newdata.c | 397 +++++++++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 496 insertions(+), 43 deletions(-) create mode 100644 tests/newdata.c diff --git a/libelf/ChangeLog b/libelf/ChangeLog index 6699052..c43a7fe 100644 --- a/libelf/ChangeLog +++ b/libelf/ChangeLog @@ -1,5 +1,14 @@ 2015-01-20 Mark Wielaard + * libelfP.h (__elf_strptr_internal): New function declaration. + * elf_getdata.c (__libelf_set_data_list_rdlock): New internal + function extracted from... + (__elf_getdata_rdlock): ... here. + * elf_newdata.c (elf_newdata): Check scn->rawdata_base and update + datalist if necessary. + +2015-01-20 Mark Wielaard + * elf_strptr.c (elf_strptr): Call __elf[32|64]_getshdr_rdlock if necessary. diff --git a/libelf/elf_getdata.c b/libelf/elf_getdata.c index 0aeb997..e8f022f 100644 --- a/libelf/elf_getdata.c +++ b/libelf/elf_getdata.c @@ -337,6 +337,44 @@ __libelf_set_rawdata (Elf_Scn *scn) return result; } +void +internal_function +__libelf_set_data_list_rdlock (Elf_Scn *scn, int wrlocked) +{ + if (scn->rawdata.d.d_buf != NULL && scn->rawdata.d.d_size > 0) + { + Elf *elf = scn->elf; + + /* Upgrade the lock to a write lock if necessary and check + nobody else already did the work. */ + if (!wrlocked) + { + rwlock_unlock (elf->lock); + rwlock_wrlock (elf->lock); + if (scn->data_list_rear != NULL) + return; + } + + /* Convert according to the version and the type. */ + convert_data (scn, __libelf_version, elf->class, + (elf->class == ELFCLASS32 + || (offsetof (struct Elf, state.elf32.ehdr) + == offsetof (struct Elf, state.elf64.ehdr)) + ? elf->state.elf32.ehdr->e_ident[EI_DATA] + : elf->state.elf64.ehdr->e_ident[EI_DATA]), + scn->rawdata.d.d_size, scn->rawdata.d.d_type); + } + else + { + /* This is an empty or NOBITS section. There is no buffer but + the size information etc is important. */ + scn->data_list.data.d = scn->rawdata.d; + scn->data_list.data.s = scn; + } + + scn->data_list_rear = &scn->data_list; +} + Elf_Data * internal_function __elf_getdata_rdlock (scn, data) @@ -427,42 +465,10 @@ __elf_getdata_rdlock (scn, data) empty in case the section has size zero (for whatever reason). Now create the converted data in case this is necessary. */ if (scn->data_list_rear == NULL) - { - if (scn->rawdata.d.d_buf != NULL && scn->rawdata.d.d_size > 0) - { - if (!locked) - { - rwlock_unlock (elf->lock); - rwlock_wrlock (elf->lock); - if (scn->data_list_rear != NULL) - goto pass; - } - - /* Convert according to the version and the type. */ - convert_data (scn, __libelf_version, elf->class, - (elf->class == ELFCLASS32 - || (offsetof (struct Elf, state.elf32.ehdr) - == offsetof (struct Elf, state.elf64.ehdr)) - ? elf->state.elf32.ehdr->e_ident[EI_DATA] - : elf->state.elf64.ehdr->e_ident[EI_DATA]), - scn->rawdata.d.d_size, scn->rawdata.d.d_type); - } - else - { - /* This is an empty or NOBITS section. There is no buffer but - the size information etc is important. */ - scn->data_list.data.d = scn->rawdata.d; - scn->data_list.data.s = scn; - } - - scn->data_list_rear = &scn->data_list; - } + __libelf_set_data_list_rdlock (scn, locked); - /* If no data is present we cannot return any. */ - if (scn->data_list_rear != NULL) - pass: - /* Return the first data element in the list. */ - result = &scn->data_list.data.d; + /* Return the first data element in the list. */ + result = &scn->data_list.data.d; out: return result; diff --git a/libelf/elf_newdata.c b/libelf/elf_newdata.c index 90d1813..f6609a8 100644 --- a/libelf/elf_newdata.c +++ b/libelf/elf_newdata.c @@ -1,5 +1,5 @@ /* Create new, empty section data. - Copyright (C) 1998, 1999, 2000, 2001, 2002 Red Hat, Inc. + Copyright (C) 1998, 1999, 2000, 2001, 2002, 2015 Red Hat, Inc. This file is part of elfutils. Contributed by Ulrich Drepper , 1998. @@ -64,6 +64,25 @@ elf_newdata (Elf_Scn *scn) rwlock_wrlock (scn->elf->lock); + /* data_read is set when data has been read from the ELF image or + when a new section has been created by elf_newscn. If data has + been read from the ELF image, then rawdata_base will point to raw + data. If data_read has been set by elf_newscn, then rawdata_base + will be NULL. data_list_rear will be set by elf_getdata if the + data has been converted, or by this function, elf_newdata, when + new data has been added. + + Currently elf_getdata and elf_update rely on the fact that when + data_list_read is not NULL all they have to do is walk the data + list. They will ignore any (unread) raw data in that case. + + So we need to make sure the data list is setup if there is + already data available. */ + if (scn->data_read + && scn->rawdata_base != NULL + && scn->data_list_rear == NULL) + __libelf_set_data_list_rdlock (scn, 1); + if (scn->data_read && scn->data_list_rear == NULL) { /* This means the section was created by the user and this is the @@ -73,6 +92,19 @@ elf_newdata (Elf_Scn *scn) } else { + /* It would be more efficient to create new data without + reading/converting the data from the file. But then we + have to remember this. Currently elf_getdata and + elf_update rely on the fact that they don't have to + load/convert any data if data_list_rear is set. */ + if (scn->data_read == 0) + { + if (__libelf_set_rawdata_wrlock (scn) != 0) + /* Something went wrong. The error value is already set. */ + goto out; + __libelf_set_data_list_rdlock (scn, 1); + } + /* Create a new, empty data descriptor. */ result = (Elf_Data_List *) calloc (1, sizeof (Elf_Data_List)); if (result == NULL) @@ -82,11 +114,6 @@ elf_newdata (Elf_Scn *scn) } result->flags = ELF_F_DIRTY | ELF_F_MALLOCED; - - if (scn->data_list_rear == NULL) - /* We create new data without reading/converting the data from the - file. That is fine but we have to remember this. */ - scn->data_list_rear = &scn->data_list; } /* Set the predefined values. */ diff --git a/libelf/libelfP.h b/libelf/libelfP.h index 3b24e75..0ad4071 100644 --- a/libelf/libelfP.h +++ b/libelf/libelfP.h @@ -532,6 +532,12 @@ extern Elf_Data *__elf_getdata_rdlock (Elf_Scn *__scn, Elf_Data *__data) internal_function; extern Elf_Data *__elf_rawdata_internal (Elf_Scn *__scn, Elf_Data *__data) attribute_hidden; +/* Should be called to setup first section data element if + data_list_rear is NULL and we know data_read is set and there is + raw data available. Might upgrade the ELF lock from a read to a + write lock. If the lock is already a write lock set wrlocked. */ +extern void __libelf_set_data_list_rdlock (Elf_Scn *scn, int wrlocked) + attribute_hidden; extern char *__elf_strptr_internal (Elf *__elf, size_t __index, size_t __offset) attribute_hidden; extern Elf_Data *__elf32_xlatetom_internal (Elf_Data *__dest, diff --git a/tests/ChangeLog b/tests/ChangeLog index f94d9be..d41d0e1 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,5 +1,12 @@ 2015-01-20 Mark Wielaard + * Makefile.am (check_PROGRAMS): Add newdata. + (TESTS): Likewise. + (newdata_LDADD): new variable. + * newdata.c: New test. + +2015-01-20 Mark Wielaard + * strptr.c: New file. * run-strptr.sh: New test. * Makefile.am (check_PROGRAMS): Add strptr. diff --git a/tests/Makefile.am b/tests/Makefile.am index c3364a2..4420e8b 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -51,7 +51,7 @@ check_PROGRAMS = arextract arsymtest newfile saridx scnnames sectiondump \ dwfl-report-elf-align varlocs backtrace backtrace-child \ backtrace-data backtrace-dwarf debuglink debugaltlink \ buildid deleted deleted-lib.so aggregate_size vdsosyms \ - getsrc_die strptr + getsrc_die strptr newdata asm_TESTS = asm-tst1 asm-tst2 asm-tst3 asm-tst4 asm-tst5 \ asm-tst6 asm-tst7 asm-tst8 asm-tst9 @@ -113,7 +113,7 @@ TESTS = run-arextract.sh run-arsymtest.sh newfile test-nlist \ run-backtrace-demangle.sh run-stack-d-test.sh run-stack-i-test.sh \ run-readelf-dwz-multi.sh run-allfcts-multi.sh run-deleted.sh \ run-linkmap-cut.sh run-aggregate-size.sh vdsosyms run-readelf-A.sh \ - run-getsrc-die.sh run-strptr.sh + run-getsrc-die.sh run-strptr.sh newdata if !BIARCH export ELFUTILS_DISABLE_BIARCH = 1 @@ -426,6 +426,7 @@ aggregate_size_LDADD = $(libdw) $(libelf) vdsosyms_LDADD = $(libdw) $(libelf) getsrc_die_LDADD = $(libdw) $(libelf) strptr_LDADD = $(libelf) +newdata_LDADD = $(libelf) if GCOV check: check-am coverage diff --git a/tests/newdata.c b/tests/newdata.c new file mode 100644 index 0000000..99952f2 --- /dev/null +++ b/tests/newdata.c @@ -0,0 +1,397 @@ +/* Test program for elf_newdata function. + Copyright (C) 2015 Red Hat, Inc. + This file is part of elfutils. + + This file is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + elfutils is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#ifdef HAVE_CONFIG_H +# include +#endif + +#include +#include +#include +#include +#include +#include +#include + +#include ELFUTILS_HEADER(elf) +#include + +// Random data string (16 bytes). +static char *DATA = "123456789ABCDEF"; +static size_t DATA_LEN = 16; + +static void +add_section_data (Elf *elf, char *buf, size_t len) +{ + printf ("Adding %zd bytes.\n", len); + + Elf_Scn *scn = elf_getscn (elf, 1); + if (scn == NULL) + { + printf ("couldn't get data section: %s\n", elf_errmsg (-1)); + exit (1); + } + + Elf_Data *data = elf_newdata (scn); + if (data == NULL) + { + printf ("cannot create newdata for section: %s\n", elf_errmsg (-1)); + exit (1); + } + + data->d_buf = buf; + data->d_type = ELF_T_BYTE; + data->d_size = len; + data->d_align = 1; + data->d_version = EV_CURRENT; + + // Let the library compute the internal structure information. + if (elf_update (elf, ELF_C_NULL) < 0) + { + printf ("failure in elf_update(NULL): %s\n", elf_errmsg (-1)); + exit (1); + } + +} + +static Elf * +create_elf (int fd, int class) +{ + Elf *elf = elf_begin (fd, ELF_C_WRITE, NULL); + if (elf == NULL) + { + printf ("cannot create ELF descriptor: %s\n", elf_errmsg (-1)); + exit (1); + } + + // Create an ELF header. + if (gelf_newehdr (elf, class) == 0) + { + printf ("cannot create ELF header: %s\n", elf_errmsg (-1)); + exit (1); + } + + GElf_Ehdr ehdr_mem; + GElf_Ehdr *ehdr = gelf_getehdr (elf, &ehdr_mem); + if (ehdr == NULL) + { + printf ("cannot get ELF header: %s\n", elf_errmsg (-1)); + exit (1); + } + + // Initialize header. + ehdr->e_ident[EI_DATA] = ELFCLASS32 ? ELFDATA2LSB : ELFDATA2MSB; + ehdr->e_ident[EI_OSABI] = ELFOSABI_GNU; + ehdr->e_type = ET_NONE; + ehdr->e_machine = class == ELFCLASS32 ? EM_PPC : EM_X86_64; + ehdr->e_version = EV_CURRENT; + + // Update the ELF header. + if (gelf_update_ehdr (elf, ehdr) == 0) + { + printf ("cannot update ELF header: %s\n", elf_errmsg (-1)); + exit (1); + } + + // Create a section. + Elf_Scn *scn = elf_newscn (elf); + if (scn == NULL) + { + printf ("cannot create new section: %s\n", elf_errmsg (-1)); + exit (1); + } + + GElf_Shdr shdr_mem; + GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_mem); + if (shdr == NULL) + { + printf ("cannot get header for data section: %s\n", elf_errmsg (-1)); + exit (1); + } + + shdr->sh_type = SHT_PROGBITS; + shdr->sh_flags = 0; + shdr->sh_addr = 0; + shdr->sh_link = SHN_UNDEF; + shdr->sh_info = SHN_UNDEF; + shdr->sh_addralign = 1; + shdr->sh_entsize = 1; + shdr->sh_name = 0; + + // Finish section, update the header. + if (gelf_update_shdr (scn, shdr) == 0) + { + printf ("cannot update header for DATA section: %s\n", elf_errmsg (-1)); + exit (1); + } + + // Add some data to the section. + add_section_data (elf, DATA, DATA_LEN); + + // Write everything to disk. + if (elf_update (elf, ELF_C_WRITE) < 0) + { + printf ("failure in elf_update(WRITE): %s\n", elf_errmsg (-1)); + exit (1); + } + + return elf; +} + +static Elf * +read_elf (int fd) +{ + printf ("Reading ELF file\n"); + Elf *elf = elf_begin (fd, ELF_C_RDWR, NULL); + if (elf == NULL) + { + printf ("cannot create ELF descriptor read-again: %s\n", elf_errmsg (-1)); + exit (1); + } + + return elf; +} + +static void +check_section_size (Elf *elf, size_t size) +{ + Elf_Scn *scn = elf_getscn (elf, 1); + if (scn == NULL) + { + printf ("couldn't get data section: %s\n", elf_errmsg (-1)); + exit (1); + } + + GElf_Shdr shdr_mem; + GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_mem); + if (shdr == NULL) + { + printf ("cannot get header for DATA section: %s\n", elf_errmsg (-1)); + exit (1); + } + + if (shdr->sh_size == size) + printf ("OK %zd bytes.\n", size); + else + { + printf ("BAD size, expected %zd, got %" PRIu64 "\n", + size, shdr->sh_size); + exit (-1); + } +} + +static void +check_section_data (Elf *elf, char *data, size_t len, size_t times) +{ + Elf_Scn *scn = elf_getscn (elf, 1); + if (scn == NULL) + { + printf ("couldn't get data section: %s\n", elf_errmsg (-1)); + exit (1); + } + + Elf_Data *d = NULL; + for (size_t i = 0; i < times; i++) + { + if (d == NULL || i * len >= d->d_off + d->d_size) + { + d = elf_getdata (scn, d); + if (d == NULL) + { + printf ("cannot get data for section item %zd: %s\n", + i, elf_errmsg (-1)); + exit (1); + } + else + printf ("OK, section data item %zd (d_off: %zd, d_size: %zd)\n", + i, d->d_off, d->d_size); + } + char *d_data = (char *) d->d_buf + (len * i) - d->d_off; + printf ("%zd data (d_off: %zd, len * i: %zd): (%p + %zd) %s\n", + i, d->d_off, len * i, d->d_buf, (len * i) - d->d_off, d_data); + if (memcmp (data, d_data, len) != 0) + { + printf ("Got bad data in section for item %zd.\n", i); + exit (1); + } + } +} + +static void +check_elf (int class) +{ + static const char *fname; + fname = class == ELFCLASS32 ? "newdata.elf32" : "newdata.elf64"; + + printf ("check_elf: %s\n", fname); + + int fd = open (fname, O_RDWR|O_CREAT|O_TRUNC, 00666); + if (fd == -1) + { + printf ("cannot create `%s': %s\n", fname, strerror (errno)); + exit (1); + } + + Elf *elf = create_elf (fd, class); + check_section_size (elf, DATA_LEN); + check_section_data (elf, DATA, DATA_LEN, 1); + + // Add some more data (won't be written to disk). + add_section_data (elf, DATA, DATA_LEN); + check_section_size (elf, 2 * DATA_LEN); + check_section_data (elf, DATA, DATA_LEN, 2); + + if (elf_end (elf) != 0) + { + printf ("failure in elf_end: %s\n", elf_errmsg (-1)); + exit (1); + } + + close (fd); + + // Read the ELF from disk now. And add new data directly. + fd = open (fname, O_RDONLY); + if (fd == -1) + { + printf ("cannot open `%s' read-only: %s\n", fname, strerror (errno)); + exit (1); + } + + elf = read_elf (fd); + check_section_size (elf, DATA_LEN); + // But don't check contents, that would read the data... + + // Add some more data. + add_section_data (elf, DATA, DATA_LEN); + check_section_size (elf, 2 * DATA_LEN); + check_section_data (elf, DATA, DATA_LEN, 2); + + // And some more. + add_section_data (elf, DATA, DATA_LEN); + check_section_size (elf, 3 * DATA_LEN); + check_section_data (elf, DATA, DATA_LEN, 3); + + if (elf_end (elf) != 0) + { + printf ("failure in elf_end: %s\n", elf_errmsg (-1)); + exit (1); + } + + close (fd); + + // Read the ELF from disk now. And add new data after raw reading. + fd = open (fname, O_RDONLY); + if (fd == -1) + { + printf ("cannot open `%s' read-only: %s\n", fname, strerror (errno)); + exit (1); + } + + elf = read_elf (fd); + check_section_size (elf, DATA_LEN); + // But don't check contents, that would read the data... + + // Get raw data before adding new data. + Elf_Scn *scn = elf_getscn (elf, 1); + if (scn == NULL) + { + printf ("couldn't get data section: %s\n", elf_errmsg (-1)); + exit (1); + } + + printf ("elf_rawdata\n"); + Elf_Data *data = elf_rawdata (scn, NULL); + if (data == NULL) + { + printf ("couldn't get raw data from section: %s\n", elf_errmsg (-1)); + exit (1); + } + + if (data->d_size != DATA_LEN) + { + printf ("Unexpected Elf_Data: %zd", data->d_size); + exit (1); + } + + // Now add more data. + add_section_data (elf, DATA, DATA_LEN); + check_section_size (elf, 2 * DATA_LEN); + check_section_data (elf, DATA, DATA_LEN, 2); + + // And some more. + add_section_data (elf, DATA, DATA_LEN); + check_section_size (elf, 3 * DATA_LEN); + check_section_data (elf, DATA, DATA_LEN, 3); + + if (elf_end (elf) != 0) + { + printf ("failure in elf_end: %s\n", elf_errmsg (-1)); + exit (1); + } + + close (fd); + + // Read the ELF from disk now. And add new data after data reading. + fd = open (fname, O_RDONLY); + if (fd == -1) + { + printf ("cannot open `%s' read-only: %s\n", fname, strerror (errno)); + exit (1); + } + + elf = read_elf (fd); + check_section_size (elf, DATA_LEN); + // Get (converted) data before adding new data. + check_section_data (elf, DATA, DATA_LEN, 1); + + printf ("elf_getdata\n"); + + // Now add more data. + add_section_data (elf, DATA, DATA_LEN); + check_section_size (elf, 2 * DATA_LEN); + check_section_data (elf, DATA, DATA_LEN, 2); + + // And some more. + add_section_data (elf, DATA, DATA_LEN); + check_section_size (elf, 3 * DATA_LEN); + check_section_data (elf, DATA, DATA_LEN, 3); + + if (elf_end (elf) != 0) + { + printf ("failure in elf_end: %s\n", elf_errmsg (-1)); + exit (1); + } + + close (fd); + + unlink (fname); +} + +int +main (int argc __attribute__ ((unused)), char *argv[] __attribute__ ((unused))) +{ + // Initialize libelf. + elf_version (EV_CURRENT); + + // Fill holes with something non-zero to more easily spot bad data. + elf_fill ('X'); + + check_elf (ELFCLASS32); + check_elf (ELFCLASS64); + + return 0; +} -- 1.8.3.1