[PATCH] ld.so: command argument "--preload"

classic Classic list List threaded Threaded
20 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] ld.so: command argument "--preload"

David Newall-3
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<---

--- elf/rtld.c.orig 2018-08-22 22:14:20.584489324 +0930
+++ elf/rtld.c 2018-08-22 22:17:22.151716644 +0930
@@ -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);

Reply | Threaded
Open this post in threaded view
|

[PATCH] ld.so: command argument "--preload"

David Newall
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<---

--- elf/rtld.c.orig 2018-08-22 22:14:20.584489324 +0930
+++ elf/rtld.c 2018-08-22 22:17:22.151716644 +0930
@@ -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);

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ld.so: command argument "--preload"

Carlos O'Donell-6
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.

--
Cheers,
Carlos.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ld.so: command argument "--preload"

H.J. Lu-30
On Fri, Aug 31, 2018 at 12:29 PM, Carlos O'Donell <[hidden email]> 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?

--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ld.so: command argument "--preload"

Carlos O'Donell-6
On 08/31/2018 04:15 PM, H.J. Lu wrote:

> On Fri, Aug 31, 2018 at 12:29 PM, Carlos O'Donell <[hidden email]> 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.

--
Cheers,
Carlos.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ld.so: command argument "--preload"

David Newall
In reply to this post by David Newall
Ping! (Copyright assignment now complete.)



-------- Forwarded Message --------
Subject: [PATCH] ld.so: command argument "--preload"
Date: Sat, 1 Sep 2018 00:16:30 +0930
From: David Newall <[hidden email]>
To: [hidden email]



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<---

--- elf/rtld.c.orig 2018-08-22 22:14:20.584489324 +0930
+++ elf/rtld.c 2018-08-22 22:17:22.151716644 +0930
@@ -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);

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ld.so: command argument "--preload"

Florian Weimer-5
In reply to this post by David Newall
* 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
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ld.so: command argument "--preload"

David Newall
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.)

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ld.so: command argument "--preload"

Carlos O'Donell-6
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.
 

--
Cheers,
Carlos.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ld.so: command argument "--preload"

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)

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

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ld.so: command argument "--preload"

Florian Weimer-5
* 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
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ld.so: command argument "--preload"

David Newall
On 6/11/18 11:14 pm, Florian Weimer wrote:
> 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.

I've done all that.

Here's a patch that adds --preload to rtld, and  includes a test that
works with or without --enable-hardcoded-path-in-tests.

By the way, is this a list which permits MIME attachments?

----8<--------8<--------8<--------8<--------8<--------8<--------8<--------8<--
diff --git a/elf/Makefile b/elf/Makefile
index d72e7b6..901e188 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -353,7 +353,8 @@ endif
 
  ifeq (yes,$(build-shared))
  ifeq ($(run-built-tests),yes)
-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
  endif
  tests-special += $(objpfx)check-textrel.out $(objpfx)check-execstack.out \
  $(objpfx)check-localplt.out $(objpfx)check-initfini.out
@@ -882,6 +883,13 @@ $(objpfx)tst-rtld-load-self.out: tst-rtld-load-self.sh $(objpfx)ld.so
  $(SHELL) $^ '$(test-wrapper)' '$(test-wrapper-env)' > $@; \
  $(evaluate-test)
 
+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)' '$(run_program_env)' \
+    '$(rpath-link)' '$(tst-rtld-preload-OBJS)' > $@; \
+ $(evaluate-test)
+
  $(objpfx)initfirst: $(libdl)
  $(objpfx)initfirst.out: $(objpfx)firstobj.so
 
diff --git a/elf/rtld.c b/elf/rtld.c
index 1b0c747..f046f6e 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -826,6 +826,8 @@ static const char *library_path attribute_relro;
  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
@@ -833,8 +835,9 @@ static int version_info attribute_relro;
     (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;
@@ -858,7 +861,7 @@ handle_ld_preload (const char *preloadlist, struct link_map *main_map)
  ++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;
  }
@@ -978,6 +981,13 @@ dl_main (const ElfW(Phdr) *phdr,
     _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;
 
@@ -1006,7 +1016,8 @@ of this helper program; chances are you did not intend to run this program.\n\
  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;
@@ -1620,7 +1631,16 @@ ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
    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);
diff --git a/elf/tst-rtld-preload.sh b/elf/tst-rtld-preload.sh
new file mode 100755
index 0000000..4a6a58d
--- /dev/null
+++ b/elf/tst-rtld-preload.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+# Test how rtld loads itself.
+# Copyright (C) 2012-2016 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+#
+
+# The GNU C Library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+
+# The GNU C Library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+
+# You should have received a copy of the GNU Lesser General Public
+# License along with the GNU C Library; if not, see
+#. + +set -e + +rtld=$1 +test_program=$2 +test_wrapper=$3
+test_wrapper_env=$4 +run_program_env=$5 +library_path=$6 +preload=$7 +
+echo "# [${test_wrapper}] [$rtld] [--library-path] [$library_path]
[--preload] [$preload] [$test_program]" +${test_wrapper_env} \
+${run_program_env} \ +${test_wrapper} $rtld --library-path
$library_path --preload $preload $test_program 2>&1 && rc=0 || rc=$?
+echo "# exit status $rc" + +exit $rc

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ld.so: command argument "--preload"

Florian Weimer-5
* David Newall:

> On 6/11/18 11:14 pm, Florian Weimer wrote:
>> 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.
>
> I've done all that.
>
> Here's a patch that adds --preload to rtld, and  includes a test that
> works with or without --enable-hardcoded-path-in-tests.
>
> By the way, is this a list which permits MIME attachments?

Yes, as long as it's not text/html.  Please resend your patch as an
attachment; it looks like some lines were wrapped.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ld.so: command argument "--preload"

David Newall
On 7/11/18 8:53 pm, Florian Weimer wrote:
> Yes, as long as it's not text/html.  Please resend your patch as an
> attachment; it looks like some lines were wrapped.

I like HTML email (I know! and I've been doing UNIX since '80s) but this
client is doing the movement no favour by mangling what I asked to be
pre-formatted text.  Here it is as an attachment.


master-preload.diff (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ld.so: command argument "--preload"

Florian Weimer-5
* David Newall:

> On 7/11/18 8:53 pm, Florian Weimer wrote:
>> Yes, as long as it's not text/html.  Please resend your patch as an
>> attachment; it looks like some lines were wrapped.
>
> I like HTML email (I know! and I've been doing UNIX since '80s) but
> this client is doing the movement no favour by mangling what I asked
> to be pre-formatted text.  Here it is as an attachment.

Much better, thanks.

> diff --git a/elf/Makefile b/elf/Makefile
> index d72e7b6..901e188 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -353,7 +353,8 @@ endif
>  
>  ifeq (yes,$(build-shared))
>  ifeq ($(run-built-tests),yes)
> -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
>  endif
>  tests-special += $(objpfx)check-textrel.out $(objpfx)check-execstack.out \
>   $(objpfx)check-localplt.out $(objpfx)check-initfini.out
> @@ -882,6 +883,13 @@ $(objpfx)tst-rtld-load-self.out: tst-rtld-load-self.sh $(objpfx)ld.so
>   $(SHELL) $^ '$(test-wrapper)' '$(test-wrapper-env)' > $@; \
>   $(evaluate-test)
>  
> +tst-rtld-preload-OBJS =  $(subst $(empty) ,:,$(strip $(preloadtest-preloads:=.so)))
> +$(objpfx)tst-rtld-preload.out: tst-rtld-preload.sh $(objpfx)ld.so \
> +       $(objpfx)preloadtest

Missing dependency on the objects in $(tst-rtld-preload-OBJS), I think.
I expect you need to add the prefix $(objpfx) to the dependency.

> + $(SHELL) $^ '$(test-wrapper)' '$(test-wrapper-env)' '$(run_program_env)' \
> +    '$(rpath-link)' '$(tst-rtld-preload-OBJS)' > $@; \
> + $(evaluate-test)

There are two whitespace errors on this line (leading and trailing
space).

>  $(objpfx)initfirst: $(libdl)
>  $(objpfx)initfirst.out: $(objpfx)firstobj.so
>  
> diff --git a/elf/rtld.c b/elf/rtld.c
> index 1b0c747..f046f6e 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -826,6 +826,8 @@ static const char *library_path attribute_relro;
>  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
> @@ -833,8 +835,9 @@ static int version_info attribute_relro;
>     (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 */

I would merge the last line into the existing comment, and there should
be a ".  " at the end (period followed by two spaces, as before).

>  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)

Please wrap the at 79 characters.

Rest looks good to me.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ld.so: command argument "--preload"

David Newall
On 8/11/18 12:02 am, Florian Weimer wrote:
>>  
>> +tst-rtld-preload-OBJS =  $(subst $(empty) ,:,$(strip $(preloadtest-preloads:=.so)))
>> +$(objpfx)tst-rtld-preload.out: tst-rtld-preload.sh $(objpfx)ld.so \
>> +       $(objpfx)preloadtest
> Missing dependency on the objects in $(tst-rtld-preload-OBJS), I think.
I think not.  This is the same as preloadtest-ENV (without
"LD_PRELOAD=").  Is it okay?  Should I have included a comment to
explain this?
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ld.so: command argument "--preload"

Florian Weimer-5
* David Newall:

> On 8/11/18 12:02 am, Florian Weimer wrote:
>>>   +tst-rtld-preload-OBJS =  $(subst $(empty) ,:,$(strip
>>> $(preloadtest-preloads:=.so)))
>>> +$(objpfx)tst-rtld-preload.out: tst-rtld-preload.sh $(objpfx)ld.so \
>>> +       $(objpfx)preloadtest
>> Missing dependency on the objects in $(tst-rtld-preload-OBJS), I think.

> I think not.  This is the same as preloadtest-ENV (without
> "LD_PRELOAD=").  Is it okay?  Should I have included a comment to
> explain this?

preloadtest.out has this:

$(objpfx)preloadtest.out: $(preloadtest-preloads:%=$(objpfx)%.so)
preloadtest-ENV = \
  LD_PRELOAD=$(subst $(empty) ,:,$(strip $(preloadtest-preloads:=.so)))

So I think you should add $(preloadtest-preloads:%=$(objpfx)%.so) as
well.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ld.so: command argument "--preload"

David Newall
On 8/11/18 12:36 am, Florian Weimer wrote:
> preloadtest.out has this:
>
> $(objpfx)preloadtest.out: $(preloadtest-preloads:%=$(objpfx)%.so)
> preloadtest-ENV = \
>    LD_PRELOAD=$(subst $(empty) ,:,$(strip $(preloadtest-preloads:=.so)))
>
> So I think you should add $(preloadtest-preloads:%=$(objpfx)%.so) as
> well.

Oh, I see, yes, you are right.  Interesting that preloadtest.out does
not depend on preloadtest.

This is a disincentive to use of $^ in the recipe. It should be like this?

tst-rtld-preload-OBJS = $(subst $(empty) ,:,$(strip $(preloadtest-preloads:=.so)))
$(objpfx)tst-rtld-preload.out: tst-rtld-preload.sh $(objpfx)ld.so \
                                $(objpfx)preloadtest \
                                $(preloadtest-preloads:%=$(objpfx)%.so)
         $(SHELL) $< $(objpfx)ld.so $(objpfx)preloadtest \
                     '$(test-wrapper)' '$(test-wrapper-env)' '$(run_program_env)' \
                     '$(rpath-link)' '$(tst-rtld-preload-OBJS)' > $@; \
         $(evaluate-test)

(or be explicit instead of $<)

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ld.so: command argument "--preload"

Florian Weimer-5
* David Newall:

> On 8/11/18 12:36 am, Florian Weimer wrote:
>> preloadtest.out has this:
>>
>> $(objpfx)preloadtest.out: $(preloadtest-preloads:%=$(objpfx)%.so)
>> preloadtest-ENV = \
>>    LD_PRELOAD=$(subst $(empty) ,:,$(strip $(preloadtest-preloads:=.so)))
>>
>> So I think you should add $(preloadtest-preloads:%=$(objpfx)%.so) as
>> well.
>
> Oh, I see, yes, you are right.  Interesting that preloadtest.out does
> not depend on preloadtest.
>
> This is a disincentive to use of $^ in the recipe. It should be like this?
>
> tst-rtld-preload-OBJS = $(subst $(empty) ,:,$(strip $(preloadtest-preloads:=.so)))
> $(objpfx)tst-rtld-preload.out: tst-rtld-preload.sh $(objpfx)ld.so \
>                                $(objpfx)preloadtest \
>                                $(preloadtest-preloads:%=$(objpfx)%.so)
>         $(SHELL) $< $(objpfx)ld.so $(objpfx)preloadtest \
>                     '$(test-wrapper)' '$(test-wrapper-env)' '$(run_program_env)' \
>                     '$(rpath-link)' '$(tst-rtld-preload-OBJS)' > $@; \
>         $(evaluate-test)
>
> (or be explicit instead of $<)

Yes, you will have to use $<.  I do not have a strong preference ($< vs
tst-rtld-preload.sh).  The test runs in the source tree, so both are
okay.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

[PATCH] ld.so: command argument "--preload"

David Newall
Thanks, Florian and Carlos, for your guidance.  This is my patch,
relative to master source, as pulled two days ago.

master-preload.diff (5K) Download Attachment