ld.so: command argument "--preload"
Commit Message
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<---
Comments
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
@@ -745,6 +745,8 @@
static const char *preloadlist attribute_relro;
/* Nonzero if information about versions has to be printed. */
static int version_info attribute_relro;
+/* The preload list passed as a command argument. */
+static const char *preloadarg attribute_relro;
/* The LD_PRELOAD environment variable gives list of libraries
separated by white space or colons that are loaded before the
@@ -752,8 +754,9 @@
(If the binary is running setuid all elements containing a '/' are
ignored since it is insecure.) Return the number of preloads
performed. */
+/* Ditto for --preload command argument */
unsigned int
-handle_ld_preload (const char *preloadlist, struct link_map *main_map)
+handle_preload_list (const char *preloadlist, struct link_map *main_map, const char *where)
{
unsigned int npreloads = 0;
const char *p = preloadlist;
@@ -777,7 +780,7 @@
++p;
if (dso_name_valid_for_suid (fname))
- npreloads += do_preload (fname, main_map, "LD_PRELOAD");
+ npreloads += do_preload (fname, main_map, where);
}
return npreloads;
}
@@ -902,6 +905,13 @@
_dl_argc -= 2;
_dl_argv += 2;
}
+ else if (! strcmp(_dl_argv[1], "--preload") && _dl_argc > 2)
+ {
+ preloadarg = _dl_argv[2];
+ _dl_skip_args += 2;
+ _dl_argc -= 2;
+ _dl_argv += 2;
+ }
else
break;
@@ -930,7 +940,8 @@
variable LD_LIBRARY_PATH\n\
--inhibit-rpath LIST ignore RUNPATH and RPATH information in object names\n\
in LIST\n\
- --audit LIST use objects named in LIST as auditors\n");
+ --audit LIST use objects named in LIST as auditors\n\
+ --preload LIST preload objects named in LIST\n");
++_dl_skip_args;
--_dl_argc;
@@ -1536,7 +1547,16 @@
if (__glibc_unlikely (preloadlist != NULL))
{
HP_TIMING_NOW (start);
- npreloads += handle_ld_preload (preloadlist, main_map);
+ npreloads += handle_preload_list (preloadlist, main_map, "LD_PRELOAD");
+ HP_TIMING_NOW (stop);
+ HP_TIMING_DIFF (diff, start, stop);
+ HP_TIMING_ACCUM_NT (load_time, diff);
+ }
+
+ if (__glibc_unlikely (preloadarg != NULL))
+ {
+ HP_TIMING_NOW (start);
+ npreloads += handle_preload_list (preloadarg, main_map, "--preload");
HP_TIMING_NOW (stop);
HP_TIMING_DIFF (diff, start, stop);
HP_TIMING_ACCUM_NT (load_time, diff);