This is the mail archive of the elfutils-devel@sourceware.org mailing list for the elfutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: find_aux_sym triggers a kernel heuristic


On Mon, 2014-03-10 at 14:37 -0700, Josh Stone wrote:
> On 03/10/2014 02:23 PM, Mark Wielaard wrote:
> > On Fri, 2014-03-07 at 17:39 -0800, Josh Stone wrote:
> >> Maybe this only needs to add a check that file == &mod->main
> > 
> > Yes, I think that would be the correct thing to do. Both find_dw and
> > find_symtab call __libdwfl_getelf first. So the main ELF file will
> > always be loaded through open_elf first. After mod->e_type has been set
> > it should not be set or changed again by either debug of aux file
> > opening.
> 
> Ok.  I attached my simple patch which still passes the testsuite, and
> also fixes my issue -- let me know how it works for you.

Yes, that works for me. I added an extra comment explaining the why and
added an assert to make sure our internal invariant holds. If you don't
mind adding your Signed-off-by line to the attached commit (see also the
CONTRIBUTING file) could you push it to master? Or let me know I should.

Thanks,

Mark
>From 65f8bca67b7f6b5a03b0c1789adf828b0c886220 Mon Sep 17 00:00:00 2001
From: Josh Stone <jistone@redhat.com>
Date: Tue, 11 Mar 2014 11:35:30 +0100
Subject: [PATCH] libdwfl: dwfl_module_getdwarf.c (open_elf) only (re)set mod->e_type once.

As noted in https://sourceware.org/bugzilla/show_bug.cgi?id=16676#c2 for
systemtap the heuristic used by open_elf to set the kernel Dwfl_Module
type to ET_DYN even if the underlying ELF file e_type was set to ET_EXEC
could trigger erroneously for non-kernel/non-main (debug or aux) files.
Make sure we only set the e_type of the module once when processing the
main file (when the phdrs can be trusted).
---
 libdwfl/ChangeLog              |    5 +++++
 libdwfl/dwfl_module_getdwarf.c |   26 ++++++++++++++++++++------
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 12938f3..8530787 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,8 @@
+2014-03-11  Josh Stone  <jistone@redhat.com>
+
+	* dwfl_module_getdwarf.c (open_elf): Only explicitly set
+	mod->e_type when processing the main ELF file.
+
 2014-03-04  Mark Wielaard  <mjw@redhat.com>
 
 	* libdwflP.h (struct __libdwfl_pid_arg): Moved here and renamed from
diff --git a/libdwfl/dwfl_module_getdwarf.c b/libdwfl/dwfl_module_getdwarf.c
index c4bd739..dcc1adb 100644
--- a/libdwfl/dwfl_module_getdwarf.c
+++ b/libdwfl/dwfl_module_getdwarf.c
@@ -1,5 +1,5 @@
 /* Find debugging and symbol information for a module in libdwfl.
-   Copyright (C) 2005-2012 Red Hat, Inc.
+   Copyright (C) 2005-2012, 2014 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -77,7 +77,7 @@ open_elf (Dwfl_Module *mod, struct dwfl_file *file)
       return DWFL_E (LIBELF, elf_errno ());
     }
 
-  if (mod->e_type != ET_REL)
+  if (ehdr->e_type != ET_REL)
     {
       /* In any non-ET_REL file, we compute the "synchronization address".
 
@@ -131,11 +131,25 @@ open_elf (Dwfl_Module *mod, struct dwfl_file *file)
 	}
     }
 
-  mod->e_type = ehdr->e_type;
+  /* We only want to set the module e_type explictly once derived from
+     the main ELF file (it might be changed for the kernel, because
+     that is special, see below).  open_elf is always called first for
+     the main ELF file because both find_dw and find_symtab call
+     __libdwfl_getelf first to open the main file.  So don't let debug
+     or aux files override the module e_type.  The kernel heuristic
+     below could otherwise trigger for non-kernel/non-main files since
+     for non main ELF files the phdrs might not match the actual load
+     addresses.  */
+  if (file == &mod->main)
+    {
+      mod->e_type = ehdr->e_type;
 
-  /* Relocatable Linux kernels are ET_EXEC but act like ET_DYN.  */
-  if (mod->e_type == ET_EXEC && file->vaddr != mod->low_addr)
-    mod->e_type = ET_DYN;
+      /* Relocatable Linux kernels are ET_EXEC but act like ET_DYN.  */
+      if (mod->e_type == ET_EXEC && file->vaddr != mod->low_addr)
+	mod->e_type = ET_DYN;
+    }
+  else
+    assert (mod->main.elf != NULL);
 
   return DWFL_E_NOERROR;
 }
-- 
1.7.1


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]