https://sourceware.org/bugzilla/show_bug.cgi?id=33967

            Bug ID: 33967
           Summary: elf_update might crash for 65280+ sections when
                    section zero headers aren't loaded
           Product: elfutils
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: libelf
          Assignee: unassigned at sourceware dot org
          Reporter: mark at klomp dot org
                CC: abbeyj at gmail dot com, elfutils-devel at sourceware dot 
org
  Target Milestone: ---

Originally filed as bug #33819 against debugedit.

(In reply to James Abbatiello from comment #0)
> I'm not sure if this more appropriate for debugedit or for elfutils/libelf.
> 
> If you try to modify an object file that contains 65,280 or more sections
> then debugedit will dereference a null pointer and crash.

It is two bugs, one in elfutils/libelf (the crash) and another in debugedit
which simply doesn't handle > 0xFF00 sections (it handles up to that many
sections, then skips the rest).

>  Here's an example:
> 
> ```
> $ cat testing.cpp
> template <int I>
> void foo() {
> }
> 
> #define F0 foo<__COUNTER__>();
> #define F1 F0 F0
> #define F2 F1 F1
> #define F3 F2 F2
> #define F4 F3 F3
> #define F5 F4 F4
> #define F6 F5 F5
> #define F7 F6 F6
> #define F8 F7 F7
> #define F9 F8 F8
> #define F10 F9 F9
> #define F11 F10 F10
> #define F12 F11 F11
> #define F13 F12 F12
> #define F14 F13 F13
> #define F15 F14 F14
> 
> int main() {
> //    F14  // approx 32779 sections
>     F15  // approx 65548 sections
>     return 0;
> }
> 
> $ g++ -g -c testing.cpp
> 
> $ debugedit -b from -d to testing.o
> Segmentation fault
> ```
> 
> If you comment out the `F15` line and uncomment the `F14` line then this
> will run successfully.
> 
> 
> The crash happens in libelf in elf32_updatenull.c in the
> __elf64_updatenull_wrlock function at this code:
> 
> ```
>       if (shnum >= SHN_LORESERVE)
>       {
>         /* We have to  fill in the number of sections in the header
>            of the zeroth section.  */
>         Elf_Scn *scn0 = &elf->state.ELFW(elf,LIBELFBITS).scns.data[0];
> 
>         update_if_changed (scn0->shdr.ELFW(e,LIBELFBITS)->sh_size,
>                            shnum, scn0->shdr_flags);
>       }
> ```
> 
> Here shnum will be greater than or equal to SHN_LORESERVE (0xFF00). 
> `scn0->shdr.ELFW(e,LIBELFBITS)` will resolve to `scn0->shdr.e64`.  This will
> be null and will then be dereferenced by the `->sh_size`, leading to the
> crash.
> 
> I can work around this by wrapping the update_if_changed call with `if
> (scn0->shdr.ELFW(e,LIBELFBITS))` but I'm not sure if that's the right fix or
> not.

Thanks for the example and fix. It is almost correct. The fix I am testing now
is:

diff --git a/libelf/elf32_updatenull.c b/libelf/elf32_updatenull.c
index 74c27fdaa7b5..a722a788ac9c 100644
--- a/libelf/elf32_updatenull.c
+++ b/libelf/elf32_updatenull.c
@@ -183,7 +183,9 @@ __elfw2(LIBELFBITS,updatenull_wrlock) (Elf *elf, int
*change_bop, size_t shnum)
          /* We have to  fill in the number of sections in the header
             of the zeroth section.  */
          Elf_Scn *scn0 = &elf->state.ELFW(elf,LIBELFBITS).scns.data[0];
-
+         /* Make sure section zero header is actually loaded.  */
+         if (scn0->shdr.ELFW(e,LIBELFBITS) == NULL)
+           (void) __elfw2(LIBELFBITS,getshdr_wrlock) (scn0);
          update_if_changed (scn0->shdr.ELFW(e,LIBELFBITS)->sh_size,
                             shnum, scn0->shdr_flags);
        }

For elfutils/libelf now working on a simpler testcase than debugedit.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

Reply via email to