ld.so: command argument "--preload"

  Hello all,

I'm new here, so I'm probably doing this wrong.  Please tell me what I
should be doing.

CHANGE

Add a --preload command argument to ld.so (aka rtld.c)

RATIONALE

I worked around a deficiency in a 3rd party binary, via LD_PRELOAD,
however any sub-process invoked by that binary tried (as advertised) to
load the library, too.  When I needed to run the (32-bit) binary under a
64-bit OS, I found that the sub-processes worked, but elicited an ugly
error about wrong ELF class.  What I needed was a way of preloading a
library for only the binary that I was executing, and not for any
sub-process (e.g. executed via system(3)).

I feel that a --preload argument is a minor change on top of which the
LD_PRELOAD environment variable should be built.  That is, there should
be a way to preload a library without it propagating to all sub-processes.

Here is my patch.

---8<------8<------8<------8<------8<------8<------8<------8<------8<------8<------8<------8<---
  

On 08/31/2018 10:46 AM, David Newall wrote:
> I'm new here, so I'm probably doing this wrong.  Please tell me what I
> should be doing.

The patch looks great, and it's something we don't have implemented.

Thanks! Fulfilling a need is reason #1 to implement something.

Adding HJ to TO:

HJ, did you propose a similar patch at one point?

This change rings a bell and I don't know why we wouldn't have implemented
this unless there was a technical limitation.

> CHANGE
> 
> Add a --preload command argument to ld.so (aka rtld.c)
> 
> RATIONALE
> 
> I worked around a deficiency in a 3rd party binary, via LD_PRELOAD,
> however any sub-process invoked by that binary tried (as advertised) to
> load the library, too.  When I needed to run the (32-bit) binary under a
> 64-bit OS, I found that the sub-processes worked, but elicited an ugly
> error about wrong ELF class.  What I needed was a way of preloading a
> library for only the binary that I was executing, and not for any
> sub-process (e.g. executed via system(3)).
> 
> I feel that a --preload argument is a minor change on top of which the
> LD_PRELOAD environment variable should be built.  That is, there should
> be a way to preload a library without it propagating to all sub-processes.
> 
> Here is my patch.
Your patch exceeds the ~15 lines of legally significant which means we 
need a copyright assignment in place to accept your patch. I don't see 
your name in the copyright assignments list, so I don't think you have
one, but please feel free to correct me if you do.

You can read about the Contribution Checklist here:
https://sourceware.org/glibc/wiki/Contribution%20checklist

The Copyright assignment process is documented here:
https://sourceware.org/glibc/wiki/Contribution%20checklist#FSF_copyright_Assignment

I suggest following 'request-assign.future' which would allow glibc
developers to continue accepting any of your patches from now into
the future. This makes it very easy to accept your patches. The FSF
can complete assignments digitally in many countries around the world
so the assignment process can be very quick and digital only.

If you have any questions, please don't hesitate to contact me off list.
I'm a project steward for the GNU C Library and I'd be happy to answer
any questions you have.
  

On Fri, Aug 31, 2018 at 12:29 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 08/31/2018 10:46 AM, David Newall wrote:
>> I'm new here, so I'm probably doing this wrong.  Please tell me what I
>> should be doing.
>
> The patch looks great, and it's something we don't have implemented.
>
> Thanks! Fulfilling a need is reason #1 to implement something.
>
> Adding HJ to TO:
>
> HJ, did you propose a similar patch at one point?

I ran into a ld.so --library-path problem:

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

Does this patch also address it?
  

On 08/31/2018 04:15 PM, H.J. Lu wrote:
> On Fri, Aug 31, 2018 at 12:29 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>> On 08/31/2018 10:46 AM, David Newall wrote:
>>> I'm new here, so I'm probably doing this wrong.  Please tell me what I
>>> should be doing.
>>
>> The patch looks great, and it's something we don't have implemented.
>>
>> Thanks! Fulfilling a need is reason #1 to implement something.
>>
>> Adding HJ to TO:
>>
>> HJ, did you propose a similar patch at one point?
> 
> I ran into a ld.so --library-path problem:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=22747
> 
> Does this patch also address it?
 
Thank you for the reference.

Your patch does not address the issue, but is related.

Here the author of the patch David Newall is wanting to implement
a --preload for the dyanmic loader to do the equivalent of
LD_PRELOAD but only for the binary that is about to be started
by the dyanmic loader (and not any children).

In your case we were talking about a new option to override
DT_RPATH and DT_RUNPATH e.g. --dt_rpath, --dt_runpath, to allow
you to run tests with hard-coded paths against different glibc
builds.

Thank you again for double checking the results. I couldn't
remember what option you had problems with.
  

* David Newall:

> I worked around a deficiency in a 3rd party binary, via LD_PRELOAD,
> however any sub-process invoked by that binary tried (as advertised) to
> load the library, too.  When I needed to run the (32-bit) binary under a
> 64-bit OS, I found that the sub-processes worked, but elicited an ugly
> error about wrong ELF class.  What I needed was a way of preloading a
> library for only the binary that I was executing, and not for any
> sub-process (e.g. executed via system(3)).
>
> I feel that a --preload argument is a minor change on top of which the
> LD_PRELOAD environment variable should be built.  That is, there should
> be a way to preload a library without it propagating to all
> sub-processes.

For some reason, the patch as posted does not apply to master.

I think the general approach of the patch is fine.  Could you write a
test case for it?

Thanks,
Florian
  

On 24/10/18 9:24 pm, Florian Weimer wrote:
> For some reason, the [--preload] patch as posted does not apply to master.
>
> I think the general approach of the patch is fine.  Could you write a
> test case for it?

I hate to say this, but I can't work out how to write a test case.  
Normally I'd crib from something similar, but I can't find any test 
cases for any of rtld's arguments.  (In all cases where I can find 
arguments for rtld, it's not rtld that's being tested, but something else.)
  

On 11/5/18 4:16 AM, David Newall wrote:
> On 24/10/18 9:24 pm, Florian Weimer wrote:
>> For some reason, the [--preload] patch as posted does not apply to
>> master.
>> 
>> I think the general approach of the patch is fine.  Could you write
>> a test case for it?
> 
> I hate to say this, but I can't work out how to write a test case.
> Normally I'd crib from something similar, but I can't find any test
> cases for any of rtld's arguments.  (In all cases where I can find
> arguments for rtld, it's not rtld that's being tested, but something
> else.)

Is elf/tst-rtld-load-self.sh a useful template?

You will need:

* 3 parts in elf/Makefile, a binary you're going to build for the
  test, and two shared objects that provide the same function, the
  default one exits with a non-zero exit code, and the preloading
  one which exits with a zero exit code.
* The test itself e.g. tst-rtld-args.sh, with a dependency on the
  DSOs and binary so they get built. It will be tests-special and
  have it's own rule to run the test, and pass in the right
  arguments along with an additional argument of the path to the
  binary to run.
* The script tst-rtld-args.sh will run the loader with --preload
  and try to load the new DSO, and run the test. It return 0 if
  it was able to successfully preload the DSO and interpose the
  function and you can catch that in your script and exit with
  a zero also to indicate the test passed e.g. just passing along
  the return code and printing some information.
  

On 6/11/18 12:40 pm, Carlos O'Donell wrote:
> On 11/5/18 4:16 AM, David Newall wrote:
>> ... I can't work out how to write a test case ...
> Is elf/tst-rtld-load-self.sh a useful template?

Thank you, yes, that helped.  I'll use the existing preloadtest program, 
so, for Makefile, something like this seems to work:

289c289,290
< tests-special += $(objpfx)tst-pathopt.out $(objpfx)tst-rtld-load-self.out
---
> tests-special += $(objpfx)tst-pathopt.out $(objpfx)tst-rtld-load-self.out \
>          $(objpfx)tst-rtld-preload.out
769a771,777
> 
> tst-rtld-preload-OBJS =  $(subst $(empty) ,:,$(strip $(preloadtest-preloads:=.so)))
> $(objpfx)tst-rtld-preload.out: tst-rtld-preload.sh $(objpfx)ld.so \
>                    $(objpfx)preloadtest
>     $(SHELL) $^ '$(test-wrapper)' '$(test-wrapper-env)' \
>             '$(rpath-link)' '$(tst-rtld-preload-OBJS)' > $@; \
>     $(evaluate-test)

For the script:

set -e

rtld=$1
test_program=$2
test_wrapper=$3
test_wrapper_env=$4
library_path=$5
preload=$6

echo "# [${test_wrapper}] [$rtld] [--library-path] [$library_path] [--preload] [$preload] [$test_program]"
${test_wrapper} $rtld --library-path $library_path --preload $preload $test_program 2>&1 && rc=0 || rc=$?
echo "# exit status $rc"

exit $rc

Is this reasonable?

David
  

* David Newall:

> On 6/11/18 12:40 pm, Carlos O'Donell wrote:
>> On 11/5/18 4:16 AM, David Newall wrote:
>>> ... I can't work out how to write a test case ...
>> Is elf/tst-rtld-load-self.sh a useful template?
>
> Thank you, yes, that helped.  I'll use the existing preloadtest
> program, so, for Makefile, something like this seems to work:
>
> 289c289,290
> < tests-special += $(objpfx)tst-pathopt.out $(objpfx)tst-rtld-load-self.out
> ---
>> tests-special += $(objpfx)tst-pathopt.out $(objpfx)tst-rtld-load-self.out \
>>          $(objpfx)tst-rtld-preload.out
> 769a771,777
>>
>> tst-rtld-preload-OBJS =  $(subst $(empty) ,:,$(strip $(preloadtest-preloads:=.so)))
>> $(objpfx)tst-rtld-preload.out: tst-rtld-preload.sh $(objpfx)ld.so \
>>                    $(objpfx)preloadtest
>>     $(SHELL) $^ '$(test-wrapper)' '$(test-wrapper-env)' \
>>             '$(rpath-link)' '$(tst-rtld-preload-OBJS)' > $@; \
>>     $(evaluate-test)

Please post unified diffs. 8-)

> For the script:
>
> set -e
>
> rtld=$1
> test_program=$2
> test_wrapper=$3
> test_wrapper_env=$4
> library_path=$5
> preload=$6
>
> echo "# [${test_wrapper}] [$rtld] [--library-path] [$library_path] [--preload] [$preload] [$test_program]"
> ${test_wrapper} $rtld --library-path $library_path --preload $preload $test_program 2>&1 && rc=0 || rc=$?
> echo "# exit status $rc"
>
> exit $rc
>
> Is this reasonable?

Please use the syntax in elf/tst-pathopt.sh, that is, include both
test_wrapper_env and run_program_env in the invocation (unquoted).

Don't forget to verify that the test works even if you configure glibc
with --enable-hardcoded-path-in-tests.  If not, we can disable the test
in this case.

Thanks,
Florian