* [PATCH] libelf: find 1st section instead of assuming
@ 2016-06-23 14:03 John Ogness
0 siblings, 0 replies; 5+ messages in thread
From: John Ogness @ 2016-06-23 14:03 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 3650 bytes --]
When getting section headers it is assumed that the first section
is on the first section list. However, it is possible that the
first section list only contains the zeroth section, in which
case either illegal memory access occurs or elf_nextscn()
erroneously returns NULL.
With this patch, checks are added to avoid the illegal memory
access and (if available) the second section list is looked at
to find the first section.
Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
libelf/elf32_updatenull.c | 11 +++++++++--
libelf/elf_nextscn.c | 38 +++++++++++++++++---------------------
2 files changed, 26 insertions(+), 23 deletions(-)
diff --git a/libelf/elf32_updatenull.c b/libelf/elf32_updatenull.c
index 03de032..7507062 100644
--- a/libelf/elf32_updatenull.c
+++ b/libelf/elf32_updatenull.c
@@ -180,6 +180,7 @@ __elfw2(LIBELFBITS,updatenull_wrlock) (Elf *elf, int *change_bop, size_t shnum)
if (shnum > 0)
{
+ struct Elf_Scn *scn1 = NULL;
Elf_ScnList *list;
bool first = true;
@@ -198,10 +199,16 @@ __elfw2(LIBELFBITS,updatenull_wrlock) (Elf *elf, int *change_bop, size_t shnum)
/* Go over all sections and find out how large they are. */
list = &elf->state.ELFW(elf,LIBELFBITS).scns;
+ /* Find the first section. */
+ if (list->cnt > 1)
+ scn1 = &list->data[1];
+ else if (list->next != NULL)
+ scn1 = &list->next->data[0];
+
/* Load the section headers if necessary. This loads the
headers for all sections. */
- if (list->data[1].shdr.ELFW(e,LIBELFBITS) == NULL)
- (void) __elfw2(LIBELFBITS,getshdr_wrlock) (&list->data[1]);
+ if (scn1 != NULL && scn1->shdr.ELFW(e,LIBELFBITS) == NULL)
+ (void) __elfw2(LIBELFBITS,getshdr_wrlock) (scn1);
do
{
diff --git a/libelf/elf_nextscn.c b/libelf/elf_nextscn.c
index 62cb891..d2f3e7c 100644
--- a/libelf/elf_nextscn.c
+++ b/libelf/elf_nextscn.c
@@ -41,6 +41,7 @@
Elf_Scn *
elf_nextscn (Elf *elf, Elf_Scn *scn)
{
+ Elf_ScnList *list;
Elf_Scn *result = NULL;
if (elf == NULL)
@@ -50,34 +51,29 @@ elf_nextscn (Elf *elf, Elf_Scn *scn)
if (scn == NULL)
{
- /* If no section handle is given return the first (not 0th) section. */
+ /* If no section handle is given return the first (not 0th) section.
+ Set scn to the 0th section and perform nextscn. */
if (elf->class == ELFCLASS32
|| (offsetof (Elf, state.elf32.scns)
== offsetof (Elf, state.elf64.scns)))
- {
- if (elf->state.elf32.scns.cnt > 1)
- result = &elf->state.elf32.scns.data[1];
- }
+ list = &elf->state.elf32.scns;
else
- {
- if (elf->state.elf64.scns.cnt > 1)
- result = &elf->state.elf64.scns.data[1];
- }
+ list = &elf->state.elf64.scns;
+
+ scn = &list->data[0];
}
else
+ list = scn->list;
+
+ if (scn + 1 < &list->data[list->cnt])
+ result = scn + 1;
+ else if (scn + 1 == &list->data[list->max]
+ && (list = list->next) != NULL)
{
- Elf_ScnList *list = scn->list;
-
- if (scn + 1 < &list->data[list->cnt])
- result = scn + 1;
- else if (scn + 1 == &list->data[list->max]
- && (list = list->next) != NULL)
- {
- /* If there is another element in the section list it must
- have at least one entry. */
- assert (list->cnt > 0);
- result = &list->data[0];
- }
+ /* If there is another element in the section list it must
+ have at least one entry. */
+ assert (list->cnt > 0);
+ result = &list->data[0];
}
rwlock_unlock (elf->lock);
--
2.8.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libelf: find 1st section instead of assuming
@ 2016-06-28 18:25 Mark Wielaard
0 siblings, 0 replies; 5+ messages in thread
From: Mark Wielaard @ 2016-06-28 18:25 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 426 bytes --]
On Fri, 2016-06-24 at 15:49 +0200, Mark Wielaard wrote:
> Thanks. None of our existing tests cover this. So I adapted one of them
> and created a testcase that fails before your fix and succeeds after.
> It does the same test for all combinations of ELF class 32/64 and
> ELF_C_RDWR[_MMAP] to make sure all different code paths are hit.
> I also added a ChangeLog entry.
I pushed this to master now.
Thanks,
Mark
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libelf: find 1st section instead of assuming
@ 2016-06-24 13:49 Mark Wielaard
0 siblings, 0 replies; 5+ messages in thread
From: Mark Wielaard @ 2016-06-24 13:49 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 821 bytes --]
Hi John,
On Thu, Jun 23, 2016 at 05:31:59PM +0200, John Ogness wrote:
> The situation occurs when adding sections to an existing ELF file that
> has none. You can see that in:
>
> libelf/elf_begin.c:file_read_elf()
>
> When an ELF file is opened with ELF_C_RDWR or ELF_C_RDWR_MMAP, scnmax is
> set to 1. That leads to the first section being placed on the second
> section list when elf_newscn() is called.
>
> Below is a relatively simple program to demonstrate this.
Thanks. None of our existing tests cover this. So I adapted one of them
and created a testcase that fails before your fix and succeeds after.
It does the same test for all combinations of ELF class 32/64 and
ELF_C_RDWR[_MMAP] to make sure all different code paths are hit.
I also added a ChangeLog entry.
Cheers,
Mark
[-- Attachment #2: 0001-libelf-find-1st-section-instead-of-assuming.patch --]
[-- Type: text/plain, Size: 13446 bytes --]
From bec18314652eaa6e0391bcf1b01bacc62c958f30 Mon Sep 17 00:00:00 2001
From: John Ogness <john.ogness@linutronix.de>
Date: Thu, 23 Jun 2016 16:03:58 +0200
Subject: [PATCH] libelf: find 1st section instead of assuming
When getting section headers it is assumed that the first section
is on the first section list. However, it is possible that the
first section list only contains the zeroth section, in which
case either illegal memory access occurs or elf_nextscn()
erroneously returns NULL.
With this patch, checks are added to avoid the illegal memory
access and (if available) the second section list is looked at
to find the first section.
A new test emptyfile is added that tests adding a section to
and "empty" ELF file 32/64 class with ELF_C_RDWR[_MMAP].
Signed-off-by: John Ogness <john.ogness@linutronix.de>
Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
libelf/ChangeLog | 6 +
libelf/elf32_updatenull.c | 11 +-
libelf/elf_nextscn.c | 38 +++----
tests/ChangeLog | 7 ++
tests/Makefile.am | 6 +-
tests/emptyfile.c | 277 ++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 320 insertions(+), 25 deletions(-)
create mode 100644 tests/emptyfile.c
diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index 668687d..0d8dd73 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,9 @@
+2016-06-24 John Ogness <john.ogness@linutronix.de>
+
+ * elf32_updatenull.c (updatenull_wrlock): Find first section.
+ * elf_nextscn.c (elf_nextscn): When scn is NULL start from 0th
+ section.
+
2016-04-14 Mark Wielaard <mjw@redhat.com>
* elf_compress.c (__libelf_compress): Free out_buf if deflateInit
diff --git a/libelf/elf32_updatenull.c b/libelf/elf32_updatenull.c
index 03de032..7507062 100644
--- a/libelf/elf32_updatenull.c
+++ b/libelf/elf32_updatenull.c
@@ -180,6 +180,7 @@ __elfw2(LIBELFBITS,updatenull_wrlock) (Elf *elf, int *change_bop, size_t shnum)
if (shnum > 0)
{
+ struct Elf_Scn *scn1 = NULL;
Elf_ScnList *list;
bool first = true;
@@ -198,10 +199,16 @@ __elfw2(LIBELFBITS,updatenull_wrlock) (Elf *elf, int *change_bop, size_t shnum)
/* Go over all sections and find out how large they are. */
list = &elf->state.ELFW(elf,LIBELFBITS).scns;
+ /* Find the first section. */
+ if (list->cnt > 1)
+ scn1 = &list->data[1];
+ else if (list->next != NULL)
+ scn1 = &list->next->data[0];
+
/* Load the section headers if necessary. This loads the
headers for all sections. */
- if (list->data[1].shdr.ELFW(e,LIBELFBITS) == NULL)
- (void) __elfw2(LIBELFBITS,getshdr_wrlock) (&list->data[1]);
+ if (scn1 != NULL && scn1->shdr.ELFW(e,LIBELFBITS) == NULL)
+ (void) __elfw2(LIBELFBITS,getshdr_wrlock) (scn1);
do
{
diff --git a/libelf/elf_nextscn.c b/libelf/elf_nextscn.c
index 62cb891..d2f3e7c 100644
--- a/libelf/elf_nextscn.c
+++ b/libelf/elf_nextscn.c
@@ -41,6 +41,7 @@
Elf_Scn *
elf_nextscn (Elf *elf, Elf_Scn *scn)
{
+ Elf_ScnList *list;
Elf_Scn *result = NULL;
if (elf == NULL)
@@ -50,34 +51,29 @@ elf_nextscn (Elf *elf, Elf_Scn *scn)
if (scn == NULL)
{
- /* If no section handle is given return the first (not 0th) section. */
+ /* If no section handle is given return the first (not 0th) section.
+ Set scn to the 0th section and perform nextscn. */
if (elf->class == ELFCLASS32
|| (offsetof (Elf, state.elf32.scns)
== offsetof (Elf, state.elf64.scns)))
- {
- if (elf->state.elf32.scns.cnt > 1)
- result = &elf->state.elf32.scns.data[1];
- }
+ list = &elf->state.elf32.scns;
else
- {
- if (elf->state.elf64.scns.cnt > 1)
- result = &elf->state.elf64.scns.data[1];
- }
+ list = &elf->state.elf64.scns;
+
+ scn = &list->data[0];
}
else
+ list = scn->list;
+
+ if (scn + 1 < &list->data[list->cnt])
+ result = scn + 1;
+ else if (scn + 1 == &list->data[list->max]
+ && (list = list->next) != NULL)
{
- Elf_ScnList *list = scn->list;
-
- if (scn + 1 < &list->data[list->cnt])
- result = scn + 1;
- else if (scn + 1 == &list->data[list->max]
- && (list = list->next) != NULL)
- {
- /* If there is another element in the section list it must
- have at least one entry. */
- assert (list->cnt > 0);
- result = &list->data[0];
- }
+ /* If there is another element in the section list it must
+ have at least one entry. */
+ assert (list->cnt > 0);
+ result = &list->data[0];
}
rwlock_unlock (elf->lock);
diff --git a/tests/ChangeLog b/tests/ChangeLog
index bcc296f..f555224 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,10 @@
+2016-06-24 Mark Wielaard <mjw@redhat.com>
+
+ * Makefile.am (check_PROGRAMS): Add emptyfile.
+ (TESTS): Likewise.
+ (emptyfile_LDADD): New variable.
+ * emptyfile.c: New test.
+
2016-02-09 Mark Wielaard <mjw@redhat.com>
* testfile-s390x-hash-both.bz2: New testfile.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index fedcb39..91526c0 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -53,7 +53,7 @@ check_PROGRAMS = arextract arsymtest newfile saridx scnnames sectiondump \
buildid deleted deleted-lib.so aggregate_size vdsosyms \
getsrc_die strptr newdata elfstrtab dwfl-proc-attach \
elfshphehdr elfstrmerge dwelfgnucompressed elfgetchdr \
- elfgetzdata elfputzdata zstrptr
+ elfgetzdata elfputzdata zstrptr emptyfile
asm_TESTS = asm-tst1 asm-tst2 asm-tst3 asm-tst4 asm-tst5 \
asm-tst6 asm-tst7 asm-tst8 asm-tst9
@@ -126,7 +126,8 @@ TESTS = run-arextract.sh run-arsymtest.sh newfile test-nlist \
run-elfgetchdr.sh \
run-elfgetzdata.sh run-elfputzdata.sh run-zstrptr.sh \
run-compress-test.sh \
- run-readelf-zdebug.sh run-readelf-zdebug-rel.sh
+ run-readelf-zdebug.sh run-readelf-zdebug-rel.sh \
+ emptyfile
if !BIARCH
export ELFUTILS_DISABLE_BIARCH = 1
@@ -476,6 +477,7 @@ elfgetchdr_LDADD = $(libelf) $(libdw)
elfgetzdata_LDADD = $(libelf)
elfputzdata_LDADD = $(libelf)
zstrptr_LDADD = $(libelf)
+emptyfile_LDADD = $(libelf)
# We want to test the libelf header against the system elf.h header.
# Don't include any -I CPPFLAGS.
diff --git a/tests/emptyfile.c b/tests/emptyfile.c
new file mode 100644
index 0000000..6d08624
--- /dev/null
+++ b/tests/emptyfile.c
@@ -0,0 +1,277 @@
+/* Test program for adding a section to an empty ELF file.
+ Copyright (C) 2016 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 <http://www.gnu.org/licenses/>. */
+
+
+#ifdef HAVE_CONFIG_H
+# include <config.h>
+#endif
+
+#include <errno.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include ELFUTILS_HEADER(elf)
+#include <gelf.h>
+
+
+/* Index of last string added. Returned by add_string (). */
+static size_t stridx = 0;
+
+/* Adds a string and returns the offset in the section. */
+static size_t
+add_string (Elf_Scn *scn, char *str)
+{
+ size_t lastidx = stridx;
+ size_t size = strlen (str) + 1;
+
+ Elf_Data *data = elf_newdata (scn);
+ if (data == NULL)
+ {
+ printf ("cannot create data SHSTRTAB section: %s\n", elf_errmsg (-1));
+ exit (1);
+ }
+
+ data->d_buf = str;
+ data->d_type = ELF_T_BYTE;
+ data->d_size = size;
+ data->d_align = 1;
+ data->d_version = EV_CURRENT;
+
+ stridx += size;
+ printf ("add_string: '%s', stridx: %zd, lastidx: %zd\n",
+ str, stridx, lastidx);
+ return lastidx;
+}
+
+static void
+check_elf (const char *fname, int class, int use_mmap)
+{
+ printf ("\nfname: %s\n", fname);
+ stridx = 0; // Reset strtab strings index
+
+ int fd = open (fname, O_RDWR | O_CREAT | O_TRUNC, 0666);
+ if (fd == -1)
+ {
+ printf ("cannot open `%s': %s\n", fname, strerror (errno));
+ exit (1);
+ }
+
+ Elf *elf = elf_begin (fd, use_mmap ? ELF_C_WRITE_MMAP : 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] = class == ELFCLASS64 ? ELFDATA2LSB : ELFDATA2MSB;
+ ehdr->e_ident[EI_OSABI] = ELFOSABI_GNU;
+ ehdr->e_type = ET_NONE;
+ ehdr->e_machine = EM_X86_64;
+ ehdr->e_version = EV_CURRENT;
+
+ if (gelf_update_ehdr (elf, ehdr) == 0)
+ {
+ printf ("cannot update ELF header: %s\n", elf_errmsg (-1));
+ exit (1);
+ }
+
+ // 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);
+ }
+
+ if (elf_end (elf) != 0)
+ {
+ printf ("failure in elf_end: %s\n", elf_errmsg (-1));
+ exit (1);
+ }
+
+ close (fd);
+
+ /* Reread the ELF from disk now. */
+ fd = open (fname, O_RDWR, 0666);
+ if (fd == -1)
+ {
+ printf ("cannot (re)open `%s': %s\n", fname, strerror (errno));
+ exit (1);
+ }
+
+ elf = elf_begin (fd, use_mmap ? ELF_C_RDWR_MMAP : ELF_C_RDWR, NULL);
+ if (elf == NULL)
+ {
+ printf ("cannot create ELF descriptor read-only: %s\n", elf_errmsg (-1));
+ exit (1);
+ }
+
+ // There are no sections yet.
+ if (elf_nextscn (elf, NULL) != NULL)
+ {
+ printf ("Empty elf had a section???\n");
+ exit (1);
+ }
+
+ // Create strtab section.
+ Elf_Scn *scn = elf_newscn (elf);
+ if (scn == NULL)
+ {
+ printf ("cannot create strings section: %s\n", elf_errmsg (-1));
+ exit (1);
+ }
+
+ // Add an empty string to the table as NUL entry for section zero.
+ add_string (scn, "");
+
+ GElf_Shdr shdr_mem;
+ GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_mem);
+ if (shdr == NULL)
+ {
+ printf ("cannot get header for strings section: %s\n", elf_errmsg (-1));
+ exit (1);
+ }
+
+ shdr->sh_type = SHT_STRTAB;
+ 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 = 0;
+ shdr->sh_name = add_string (scn, ".strtab");
+
+ // We have to store the section strtab index in the ELF header.
+ // So sections have actual names.
+ int ndx = elf_ndxscn (scn);
+ ehdr->e_shstrndx = ndx;
+
+ if (gelf_update_ehdr (elf, ehdr) == 0)
+ {
+ printf ("cannot update ELF header: %s\n", elf_errmsg (-1));
+ exit (1);
+ }
+
+ // Finished strtab section, update the header.
+ if (gelf_update_shdr (scn, shdr) == 0)
+ {
+ printf ("cannot update STRTAB section header: %s\n", elf_errmsg (-1));
+ exit (1);
+ }
+
+ // 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);
+ }
+
+ if (elf_end (elf) != 0)
+ {
+ printf ("failure in elf_end: %s\n", elf_errmsg (-1));
+ exit (1);
+ }
+
+ close (fd);
+
+ // And read it in one last time.
+ fd = open (fname, O_RDONLY, 0666);
+ if (fd == -1)
+ {
+ printf ("cannot open `%s' read-only: %s\n", fname, strerror (errno));
+ exit (1);
+ }
+
+ elf = elf_begin (fd, use_mmap ? ELF_C_READ_MMAP : ELF_C_READ, NULL);
+ if (elf == NULL)
+ {
+ printf ("cannot create ELF descriptor read-only: %s\n", elf_errmsg (-1));
+ exit (1);
+ }
+
+ // Is our new section there?
+ scn = elf_nextscn (elf, NULL);
+ if (scn == NULL)
+ {
+ printf ("cannot get new section: %s\n", elf_errmsg (-1));
+ exit (1);
+ }
+
+ shdr = gelf_getshdr (scn, &shdr_mem);
+ if (shdr == NULL)
+ {
+ printf ("cannot get header for new section: %s\n", elf_errmsg (-1));
+ exit (1);
+ }
+
+ size_t shstrndx;
+ if (elf_getshdrstrndx (elf, &shstrndx) < 0)
+ {
+ printf ("elf_getshdrstrndx: %s\n", elf_errmsg (-1));
+ exit (1);
+ }
+
+ const char *sname = elf_strptr (elf, shstrndx, shdr->sh_name);
+ if (sname == NULL || strcmp (sname, ".strtab") != 0)
+ {
+ printf ("Bad section name: %s\n", sname);
+ exit (1);
+ }
+
+ 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)))
+{
+ elf_version (EV_CURRENT);
+
+ check_elf ("empty.elf.32", ELFCLASS32, 0);
+ check_elf ("empty.elf.32.mmap", ELFCLASS32, 1);
+ check_elf ("empty.elf.64", ELFCLASS64, 0);
+ check_elf ("empty.elf.64.mmap", ELFCLASS64, 1);
+
+ return 0;
+}
--
2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libelf: find 1st section instead of assuming
@ 2016-06-23 15:31 John Ogness
0 siblings, 0 replies; 5+ messages in thread
From: John Ogness @ 2016-06-23 15:31 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 1281 bytes --]
Hi Mark,
On 2016-06-23, Mark Wielaard <mjw@redhat.com> wrote:
>> When getting section headers it is assumed that the first section
>> is on the first section list. However, it is possible that the
>> first section list only contains the zeroth section, in which
>> case either illegal memory access occurs or elf_nextscn()
>> erroneously returns NULL.
>>
>> With this patch, checks are added to avoid the illegal memory
>> access and (if available) the second section list is looked at
>> to find the first section.
>
> Both changes to updatenull and nextscn do make sense to me.
>
> I assume this wasn't just theoretical? I didn't immediately see how
> this situation occurs. Do you happen to have an example/testcase?
The situation occurs when adding sections to an existing ELF file that
has none. You can see that in:
libelf/elf_begin.c:file_read_elf()
When an ELF file is opened with ELF_C_RDWR or ELF_C_RDWR_MMAP, scnmax is
set to 1. That leads to the first section being placed on the second
section list when elf_newscn() is called.
Below is a relatively simple program to demonstrate this. This program
adds customized section notes to core files. It is being developed as a
feature for the minicoredumper project.
John Ogness
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: test_add_scn.c --]
[-- Type: text/x-csrc, Size: 2303 bytes --]
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <libelf.h>
#include <gelf.h>
#include <sys/types.h>
#include <sys/stat.h>
static int add_shstrtab_section(Elf *e, GElf_Off offset, GElf_Word *size)
{
GElf_Shdr shdr;
Elf_Data *data;
Elf_Scn *scn;
scn = elf_newscn(e);
if (!scn)
return -1;
data = elf_newdata(scn);
if (!data)
return -1;
data->d_align = 1;
data->d_off = 0;
data->d_buf = "\0.debug\0.shstrtab";
data->d_type = ELF_T_BYTE;
data->d_size = 18;
data->d_version = EV_CURRENT;
if (gelf_getshdr(scn, &shdr) == NULL)
return -1;
shdr.sh_name = 8;
shdr.sh_type = SHT_STRTAB;
shdr.sh_flags = SHF_STRINGS;
shdr.sh_entsize = 0;
shdr.sh_size = data->d_size;
shdr.sh_offset = offset;
shdr.sh_addralign = 1;
gelf_update_shdr(scn, &shdr);
*size = shdr.sh_size;
return 0;
}
static int add_debug_section(Elf *e, GElf_Off offset, GElf_Word size)
{
GElf_Shdr shdr;
Elf_Scn *scn;
scn = elf_newscn(e);
if (!scn)
return -1;
if (gelf_getshdr(scn, &shdr) == NULL)
return -1;
shdr.sh_name = 1;
shdr.sh_type = SHT_PROGBITS;
shdr.sh_flags = 0;
shdr.sh_entsize = 0;
shdr.sh_offset = offset;
shdr.sh_size = size;
shdr.sh_addralign = 1;
gelf_update_shdr(scn, &shdr);
return 0;
}
int main(int argc, const char *argv[])
{
size_t last_offset;
GElf_Ehdr ehdr;
GElf_Word size;
struct stat sb;
int ret;
Elf *e;
int fd;
if (elf_version(EV_CURRENT) == EV_NONE)
return 1;
fd = open(argv[1], O_RDWR);
if (fd < 0)
return 1;
e = elf_begin(fd, ELF_C_RDWR, NULL);
if (!e)
return 1;
elf_flagelf(e, ELF_C_SET, ELF_F_LAYOUT);
if (gelf_getehdr(e, &ehdr) == NULL)
return 1;
/* use file size as last offset */
if (fstat(fd, &sb) != 0)
return 1;
last_offset = sb.st_size;
/* cover everything after the elf header to ensure
* that there are no gaps for libelf to fill */
if (add_debug_section(e, ehdr.e_ehsize,
last_offset - ehdr.e_ehsize) != 0) {
return 1;
}
if (add_shstrtab_section(e, last_offset, &size) != 0)
return 1;
last_offset += size;
ehdr.e_shoff = last_offset;
ehdr.e_shstrndx = 2;
gelf_update_ehdr(e, &ehdr);
elf_flagelf(e, ELF_C_SET, ELF_F_DIRTY);
ret = elf_update(e, ELF_C_WRITE);
if (ret < 0)
return 1;
elf_end(e);
close(fd);
return 0;
}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libelf: find 1st section instead of assuming
@ 2016-06-23 15:12 Mark Wielaard
0 siblings, 0 replies; 5+ messages in thread
From: Mark Wielaard @ 2016-06-23 15:12 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 740 bytes --]
Hi John,
On Thu, 2016-06-23 at 16:03 +0200, John Ogness wrote:
> When getting section headers it is assumed that the first section
> is on the first section list. However, it is possible that the
> first section list only contains the zeroth section, in which
> case either illegal memory access occurs or elf_nextscn()
> erroneously returns NULL.
>
> With this patch, checks are added to avoid the illegal memory
> access and (if available) the second section list is looked at
> to find the first section.
Both changes to updatenull and nextscn do make sense to me.
I assume this wasn't just theoretical? I didn't immediately see how this
situation occurs. Do you happen to have an example/testcase?
Thanks,
Mark
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-06-28 18:25 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-23 14:03 [PATCH] libelf: find 1st section instead of assuming John Ogness
2016-06-23 15:12 Mark Wielaard
2016-06-23 15:31 John Ogness
2016-06-24 13:49 Mark Wielaard
2016-06-28 18:25 Mark Wielaard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).