Opened 4 years ago

Closed 4 years ago

#12287 closed bug (fixed)

runtime_loader GetSymbolCache has invalid implicit assumption

Reported by: cdurrett Owned by: bonefish
Priority: normal Milestone: Unscheduled
Component: System/runtime_loader Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

Bug because code assumes ELF being loaded was made with GCC. A yasm ELF file now fails with a segment/memory exception.

Line 61 (in current github source) in file elf_symbol_lookup.h reads:

fTableSize(image->symhash[1]),

If it is changed to read:

fTableSize(image->symhash ? image->symhash[1] | 0),

the bug goes away.

Attachments (1)

0001-runtime_loader-Do-not-assume-executable-has-dynamic-.patch (939 bytes ) - added by simonsouth 4 years ago.
Don't assume executable has dynamic segment

Download all attachments as: .zip

Change History (8)

comment:1 by waddlesplash, 4 years ago

Would you care to contribute a git-patch for this? That way you get credit for the change in Git.

in reply to:  1 comment:2 by cdurrett, 4 years ago

Replying to waddlesplash:

Would you care to contribute a git-patch for this? That way you get credit for the change in Git.

I prefer cash to credit. :-)

comment:3 by bonefish, 4 years ago

Apparently your shared object file doesn't have a dynamic section. The specs say:

"If an object file participates in dynamic linking, its program header table will have an element of type PT_DYNAMIC."

So, TBH, I'm not sure why we allow loading shared objects without a dynamic section at all. What is your shared object and what does it do?

comment:4 by cdurrett, 4 years ago

Ascend with me to flight level 300. :-)

The simplest program should work. This program for example.

; s.a section .text

global _start

_start: ret

If in the terminal I

yasm -f elf s.a ld -s -o s s.o ./s

Program 's' should do nothing - except to get loaded and executed by runtime_loader.

In all the effort done to support and optimize "shared object programs" with "dynamic sections", this simplest case looks like it was inadvertently overlooked sometime after December 2008.

I've made the less-than-one-line change mentioned in the ticket and restored the handling of the simplest case. The change also protects runtime_loader from abending in an allocation routine and thus becoming an attack vector to the rest of the OS.

Charles

by simonsouth, 4 years ago

Don't assume executable has dynamic segment

comment:5 by simonsouth, 4 years ago

Has a Patch: set

comment:6 by simonsouth, 4 years ago

I've attached a patch that implements Charles' solution (it looks correct to me), tweaked to match the coding guidelines.

What yasm is producing in his example is a non-position-independent, (effectively) statically linked executable, so the patch for #12427 should be applied as well to ensure the program is positioned correctly in memory on load.

With both these patches applied the example assembles and runs correctly for me on hrev49717.

comment:7 by pulkomandy, 4 years ago

Resolution: fixed
Status: newclosed

Applied in hrev49728. Thanks!

Note: See TracTickets for help on using tickets.