Make ld-elf.so.1 directly executable
ClosedPublic

Authored by kib on May 13 2017, 2:45 PM.

Details

Summary

The map_object.c change fixes the AT_EXECFD functionality, which is the prerequisite for the main part of the patch. If the mapped object is linked at specific address, we must obey it. Before, only in-kernel ELF image activator needed to keep the mapping address, since only binaries are linked at the fixed address, and binaries were always mapped by kernel.

For rtld.c bits, check if passed phdr is actually phdr of the interpreter itself, and decide that this is the case of direct execution. In this case, the binary to activate is specified in the argv[1]. After opening it, shift down on-stack structure with argv, env and aux vectors to emulate execution of the binary and not of the interpreter.

Right now, the calculation of execpath in the direct execution mode is too naive, it affects $ORIGIN. Might be, ld-elf.so.1 should search the binary in $PATH, but I am not sure. Anyway, this patch is a good initial step, I believe.

Test Plan

Booted multiuser on amd64, tested manually i386 ld-elf.so.1 with compat32.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kib created this revision.May 13 2017, 2:45 PM
kib edited the summary of this revision. (Show Details)May 13 2017, 2:46 PM
emaste added a subscriber: stevek.May 13 2017, 5:39 PM
emaste added inline comments.
libexec/rtld-elf/rtld.c
342–353 ↗(On Diff #28292)

Perhaps commit the re-sorting independently first?

486 ↗(On Diff #28292)

As an aside it seems O_VERIFY is not documented in open.2. I see that this came from https://github.com/freebsd/freebsd/pull/27 - paging @stevek

Also, what happens if we try to ld-elf.so.1 foo where foo has ---x--x--x perms?

kib added a comment.May 13 2017, 5:50 PM
pooma% ls -l ~/build/bsd/DEV/stuff/tests/args
---x--x--x  1 kostik  kostik  6234 Apr 18  2010 /usr/home/kostik/build/bsd/DEV/stuff/tests/args
pooma% LD_LIBRARY_PATH=/usr/lib32 obj-i386/i386.i386/usr/home/kostik/work/build/bsd/DEV/src/libexec/rtld-elf/ld-elf.so.1 ~/build/bsd/DEV/stuff/tests/args 1 2 3
Fatal error

I.e. as expected, userspace needs to read executing binary.

libexec/rtld-elf/rtld.c
342–353 ↗(On Diff #28292)

This is not just resorting, I moved locals from the nested blocks and added some variables as well.

I can produce separate patch to reorder existing local section, but the bits in the patch are not extractable.

486 ↗(On Diff #28292)

It is something internal from J that got committed but never get the flesh in our repository.

emaste added inline comments.May 13 2017, 6:04 PM
libexec/rtld-elf/rtld.c
342–353 ↗(On Diff #28292)

Right, I mean there is both resorting and other changes and I think the resorting part could be committed by itself - it would make it more clear what the other changes are.

kib updated this revision to Diff 28299.May 13 2017, 7:11 PM

Re-merge after the locals reordering changes were committed.

emaste added inline comments.May 13 2017, 7:48 PM
libexec/rtld-elf/rtld.c
481 ↗(On Diff #28299)

extra space before aux_info?

494–495 ↗(On Diff #28299)

I think this comment could be expanded a bit to separately explain main_arg[cv] and the argv/envp/auxp manipulation that follows it.

kib updated this revision to Diff 28301.May 13 2017, 8:07 PM

Update comment, remove stray space after copy.
Correct (remove) re-calculation of main_argv, it was shifted right too much.
Noted by emaste.

emaste accepted this revision.May 13 2017, 8:12 PM

This looks good to me.

This revision is now accepted and ready to land.May 13 2017, 8:12 PM
emaste added inline comments.May 14 2017, 12:22 AM
libexec/rtld-elf/rtld.c
381 ↗(On Diff #28301)

With the patch this assert triggers via

% /libexec/ld-elf.so.1 /libexec/ld-elf.so.1
ld-elf.so.1: assert failed: /usr/home/emaste/src/freebsd/libexec/rtld-elf/rtld.c:381
kib updated this revision to Diff 28326.May 14 2017, 12:36 PM

Fixes.

Move the shift before first getenv() call in the rtld, to not depend on details of getenv() implementation.
Do not forget to reset 'env' after environment is moved,not only environ (this caused env -i /libexec/ld-elf.so.1 /bin/ls segfault in jemalloc initializer).
Fix off-by-half in copying the auxv (this caused the assertion reported by Ed).

This revision now requires review to proceed.May 14 2017, 12:36 PM
emaste accepted this revision.May 15 2017, 3:05 PM
emaste added inline comments.
libexec/rtld-elf/rtld.c
444 ↗(On Diff #28326)

!= NULL?

also envp below

This revision is now accepted and ready to land.May 15 2017, 3:05 PM
kib updated this revision to Diff 28356.May 15 2017, 3:21 PM
kib marked an inline comment as done.

s/0/NULL/

This revision now requires review to proceed.May 15 2017, 3:21 PM
emaste accepted this revision.May 15 2017, 3:51 PM
This revision is now accepted and ready to land.May 15 2017, 3:51 PM
kan resigned from this revision.May 15 2017, 3:54 PM

very unlikely I'll look at this any time soon.

stevek added inline comments.May 15 2017, 5:11 PM
libexec/rtld-elf/rtld.c
486 ↗(On Diff #28292)

See D2902, which has the changes. I guess after all this time people have had enough time to comment. I'll see about committing that set of changes.

emaste added inline comments.May 15 2017, 6:48 PM
libexec/rtld-elf/rtld.c
486 ↗(On Diff #28292)

Ah. I added a comment in there - I'd suggest we should go ahead with the open.2 changes right now, since the corresponding source changes were made some time ago, and follow up with a D2902 rebase.

This revision was automatically updated to reflect the committed changes.
stevek added inline comments.May 15 2017, 7:34 PM
libexec/rtld-elf/rtld.c
486 ↗(On Diff #28292)

Sure, makes sense. I committed just the open.2 change and will rebase D2902.