[PATCH 0/8] ELF unexec (bug#20614)

[PATCH 0/8] ELF unexec (bug#20614)

[Alan Modra]

This series of patches addresses bug #20614 by rewriting the bss
handling code rather than adding yet more hacks.  Finding bss sections
by name as we do currently results in complicated code that
- does not account for all names of possible bss sections,
- assumes specific ordering of bss sections,
- can wrongly choose a SHT_NOBITS section not in the bss segment,
- incorrectly calculates bss size (no accounting for alignment gaps),
- assumes .data and .bss are in the same segment.

All of these problems and more are solved by finding the bss segment
in PT_LOAD headers, ie. the address range included in p_memsz but not
p_filesz of the last PT_LOAD header, then matching SHT_NOBITS sections
in that address range.

I've split the patch into a series as there are multiple problems of
varying severity in this code, and it might be a little easier to
review this way.  The series was tested indiviually on x86_64-linux
and on powerpc64-linux and powerpc64le-linux after fixing bug #20614
with patch 7/8.

Patch 8/8 goes further and removes the added .data2 section.  I don't
see any need for a new SHT_PROGBITS section to cover the old bss
section(s) plus dumped data.  Not adding a section means there is no
need to patch symbol st_shndx or section sh_info and sh_link fields,
which is fragile code.  At least, sh_info shouldn't really be patched
unless SHF_INFO_LINK is set in sh_flags, but not all linkers set the
flag properly.


Alan Modra (8):
  ELF unexec: Correct section header index
  ELF unexec: Tidy code
  ELF unexec: Merge Alpha and MIPS COFF debug handling
  ELF unexec: Symbol table patching
  ELF unexec: _OBJC_ symbols in bss sections
  ELF unexec: R_*_NONE relocs
  ELF unexec: Drive from PT_LOAD header rather than sections
  ELF unexec: Don't insert a new section

 src/unexelf.c | 1022 ++++++++++++---------------------------------------------
 1 file changed, 206 insertions(+), 816 deletions(-)

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 0/8] ELF unexec (bug#20614)

[Eli Zaretskii]

> Date: Mon, 2 Nov 2015 12:23:34 +1030
> From: Alan Modra <amodra@gmail.com>
> 
> This series of patches addresses bug #20614 by rewriting the bss
> handling code rather than adding yet more hacks.

Thanks.  It would be best to send these to the bug tracker,
20614@debbugs.gnu.org, so that this is recorded with the bug.

> Finding bss sections
> by name as we do currently results in complicated code that
> - does not account for all names of possible bss sections,
> - assumes specific ordering of bss sections,
> - can wrongly choose a SHT_NOBITS section not in the bss segment,
> - incorrectly calculates bss size (no accounting for alignment gaps),
> - assumes .data and .bss are in the same segment.
> 
> All of these problems and more are solved by finding the bss segment
> in PT_LOAD headers, ie. the address range included in p_memsz but not
> p_filesz of the last PT_LOAD header, then matching SHT_NOBITS sections
> in that address range.
> 
> I've split the patch into a series as there are multiple problems of
> varying severity in this code, and it might be a little easier to
> review this way.  The series was tested indiviually on x86_64-linux
> and on powerpc64-linux and powerpc64le-linux after fixing bug #20614
> with patch 7/8.
> 
> Patch 8/8 goes further and removes the added .data2 section.  I don't
> see any need for a new SHT_PROGBITS section to cover the old bss
> section(s) plus dumped data.  Not adding a section means there is no
> need to patch symbol st_shndx or section sh_info and sh_link fields,
> which is fragile code.  At least, sh_info shouldn't really be patched
> unless SHF_INFO_LINK is set in sh_flags, but not all linkers set the
> flag properly.

I'll let ELF experts review this, but in any case, we will need your
legal paperwork before we can accept such a substantial contribution.
Let me know if you want me to send you the copyright assignment form.

Thanks!

bug#20614: [PATCH 0/8] ELF unexec (bug#20614)

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 469 bytes --]

Eli Zaretskii wrote:
> Thanks.  It would be best to send these to the bug tracker,
> 20614@debbugs.gnu.org, so that this is recorded with the bug.

Done by attaching them to this email, which is CC'ed there.

> I'll let ELF experts review this, but in any case, we will need your
> legal paperwork before we can accept such a substantial contribution.

At first glance these patches look good to me; thanks Alan!  We can pick these 
up once the paperwork goes through.

[-- Attachment #2: unexec-fixes.tgz --]
[-- Type: application/x-compressed-tar, Size: 30262 bytes --]

Re: [PATCH 0/8] ELF unexec (bug#20614)

[Paul Eggert]

Thanks, Alan.  I emailed those patches to 20614@debbugs.gnu.org so that they can 
be easily found for installation once your copyright paperwork goes through.  I 
CC'ed you but gmail rejected it due to my .tgz attachment; you can see the email 
archived at:

http://bugs.gnu.org/20614#198

Re: [PATCH 0/8] ELF unexec (bug#20614)

[Alan Modra]

On Mon, Nov 02, 2015 at 05:40:43AM +0200, Eli Zaretskii wrote:
> I'll let ELF experts review this, but in any case, we will need your
> legal paperwork before we can accept such a substantial contribution.

Huh, after sending off the request for the form to assign@gnu.org,
IBM legal get back to me to say they believe IBM's FSF copyright
assignment letter does cover contributions to EMACS.

Being an IBM employee and performing this work as part of my normal
employment, I believe IBM therefore owns the work and thus legally
can and has already assigned copyright to the FSF.

If this isn't enough, I'll follow through with a personal copyright
assignment too.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 0/8] ELF unexec (bug#20614)

[John Wiegley]

>>>>> Alan Modra <amodra@gmail.com> writes:

> Huh, after sending off the request for the form to assign@gnu.org, IBM legal
> get back to me to say they believe IBM's FSF copyright assignment letter
> does cover contributions to EMACS.

> Being an IBM employee and performing this work as part of my normal
> employment, I believe IBM therefore owns the work and thus legally can and
> has already assigned copyright to the FSF.

> If this isn't enough, I'll follow through with a personal copyright
> assignment too.

At this point you should ask the GNU managers at assign@gnu.org. You can find
further notes about this here:

    https://www.gnu.org/prep/maintain/html_node/Copyright-Papers.html

John

Re: [PATCH 0/8] ELF unexec (bug#20614)

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > Huh, after sending off the request for the form to assign@gnu.org,
  > IBM legal get back to me to say they believe IBM's FSF copyright
  > assignment letter does cover contributions to EMACS.

  > Being an IBM employee and performing this work as part of my normal
  > employment, I believe IBM therefore owns the work and thus legally
  > can and has already assigned copyright to the FSF.

That should be enough.  Please send assign@gnu.org the statement that
IBM sent you, to store in our records.

Be sure to tell the Emacs maintainer if you ever cease to be an IBM
employee!
-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

bug#20614: [PATCH 0/8] ELF unexec (bug#20614)

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 441 bytes --]

Alan Modra wrote:
> IBM legal get back to me to say they believe IBM's FSF copyright
> assignment letter does cover contributions to EMACS.

This should be enough. I installed your patch into Emacs master, with one minor 
fixup (attached) to pacify './configure --enable-gcc-warnings'.

Petr, can you please verify that this fixes the bug for you? If so, I can mark 
the bug as fixed in our database.

Thanks again for fixing the bug, Alan.

[-- Attachment #2: 0001-src-unexelf.c-NEW_PROGRAM_H-Remove-unused-macro-Bug-.patch --]
[-- Type: text/x-diff, Size: 1194 bytes --]

From fc4d0bb842509ce81d2dfe2dec1f7fdbef728531 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 8 Nov 2015 09:36:14 -0800
Subject: [PATCH] * src/unexelf.c (NEW_PROGRAM_H): Remove unused macro
 (Bug#20614).

---
 src/unexelf.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/unexelf.c b/src/unexelf.c
index 85ed934..c10c7f2 100644
--- a/src/unexelf.c
+++ b/src/unexelf.c
@@ -208,8 +208,6 @@ entry_address (void *section_h, ptrdiff_t idx, ptrdiff_t entsize)
   (*(ElfW (Shdr) *) entry_address (new_section_h, n, new_file_h->e_shentsize))
 #define OLD_PROGRAM_H(n) \
   (*(ElfW (Phdr) *) entry_address (old_program_h, n, old_file_h->e_phentsize))
-#define NEW_PROGRAM_H(n) \
-  (*(ElfW (Phdr) *) entry_address (new_program_h, n, new_file_h->e_phentsize))
 
 typedef unsigned char byte;
 
@@ -250,7 +248,7 @@ unexec (const char *new_name, const char *old_name)
   ElfW (Phdr) *old_bss_seg, *new_bss_seg;
   ElfW (Addr) old_bss_addr, new_bss_addr;
   ElfW (Word) old_bss_size, new_data2_size;
-  ElfW (Off)  old_bss_offset, new_data2_offset;
+  ElfW (Off) old_bss_offset, new_data2_offset;
 
   ptrdiff_t n;
   ptrdiff_t old_bss_index;
-- 
2.1.0

bug#20614: [PATCH 0/8] ELF unexec (bug#20614)

[Lars Ingebrigtsen]

Paul Eggert <eggert@cs.ucla.edu> writes:

> This should be enough. I installed your patch into Emacs master, with
> one minor fixup (attached) to pacify './configure
> --enable-gcc-warnings'.
>
> Petr, can you please verify that this fixes the bug for you? If so, I
> can mark the bug as fixed in our database.

That was 15 weeks ago, so this is presumably working now.  I'm closing
the bug report; please reopen if this is still a problem.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no