dlsym(handle) may search in unrelated libraries

Submitted by Markus Wichmann on Feb. 6, 2019, 4:02 p.m.

Details

Message ID 20190206160248.GB5469@voyager
State New
Series "dlsym(handle) may search in unrelated libraries"
Headers show

Commit Message

Markus Wichmann Feb. 6, 2019, 4:02 p.m.
On Wed, Feb 06, 2019 at 12:02:39AM +0300, Alexey Izbyshev wrote:
> Hello!
> 
> I've discovered a bug in musl dynamic loader (tested on 1.1.21) which is
> demonstrated by the following simple example:
> 
> $ cat bar.c
> int bar = 42;
> $ musl-gcc -fPIC -shared bar.c -o libbar.so
> $ cat foo.c
> extern int bar;
> int *foo = &bar;
> $ musl-gcc -fPIC -shared foo.c -L. -lbar -Wl,-rpath='$ORIGIN' -o libfoo.so
> $ cat main.c
> #include <dlfcn.h>
> #include <stdio.h>
> 
> int main(void) {
>   if (!dlopen("libfoo.so", RTLD_NOW))
>     return 1;
>   void *h = dlopen("libc.so.6", RTLD_NOW);
>   printf("%p\n", dlsym(h, "bar"));
> }
> $ musl-gcc main.c -Wl,-rpath='$ORIGIN' -ldl
> $ ./a.out
> 0x7fd7ebe96020
> 

In case you were wondering, your typo here doesn't change anything,
since "libc.so.6" has the prefix "libc.", which is recogized as reserved
in load_library(), and makes dlopen() return a handle to the libc.

Thankfully the patch is simple: Explicitly make ldso and vdso have no
deps. I was tempted to put this into kernel_mapped_dso(), but then I
remembered that the app is also a kernel mapped dso, and it usually does
have deps that need processing. At least, in nontrivial cases.

The attached patch should tide you over.

> 
> Alexey

Ciao,
Markus

Patch hide | download patch | download mbox

From e823910d69ff56ffccecaa9b29fd4b67b901798a Mon Sep 17 00:00:00 2001
From: Markus Wichmann <nullplan@gmx.net>
Date: Wed, 6 Feb 2019 16:51:53 +0100
Subject: [PATCH 6/6] Make libc and vdso explicitly have no deps.

Alexey Izbyshev reported that without this, dlopen("libc.so") returns a
handle that is capable of finding every symbol in libraries loaded as
dependencies, since dso->deps == 0 usually means dependencies haven't
been loaded.
---
 ldso/dynlink.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ldso/dynlink.c b/ldso/dynlink.c
index ec921dfd..6ffeca85 100644
--- a/ldso/dynlink.c
+++ b/ldso/dynlink.c
@@ -1244,6 +1244,7 @@  static void reloc_all(struct dso *p)
 static void kernel_mapped_dso(struct dso *p)
 {
 	size_t min_addr = -1, max_addr = 0, cnt;
+	static const struct dso *sentinel = 0;
 	Phdr *ph = p->phdr;
 	for (cnt = p->phnum; cnt--; ph = (void *)((char *)ph + p->phentsize)) {
 		if (ph->p_type == PT_DYNAMIC) {
@@ -1428,6 +1429,7 @@  hidden void __dls2(unsigned char *base, size_t *sp)
 	ldso.phdr = laddr(&ldso, ehdr->e_phoff);
 	ldso.phentsize = ehdr->e_phentsize;
 	kernel_mapped_dso(&ldso);
+	ldso.deps = (struct dso**)&nodeps_dummy;
 	decode_dyn(&ldso);
 
 	if (DL_FDPIC) makefuncdescs(&ldso);
@@ -1675,6 +1677,7 @@  _Noreturn void __dls3(size_t *sp)
 		vdso.prev = tail;
 		tail->next = &vdso;
 		tail = &vdso;
+		vdso.deps = (struct dso**)&nodeps_dummy;
 	}
 
 	for (i=0; app.dynv[i]; i+=2) {
-- 
2.20.1


Comments

Alexey Izbyshev Feb. 6, 2019, 5:02 p.m.
On 2/6/19 7:02 PM, Markus Wichmann wrote:
> 
> Thankfully the patch is simple: Explicitly make ldso and vdso have no
> deps. I was tempted to put this into kernel_mapped_dso(), but then I
> remembered that the app is also a kernel mapped dso, and it usually does
> have deps that need processing. At least, in nontrivial cases.
> 
> The attached patch should tide you over.
> 
Thank you for the quick response and the patch, Markus! Your patch fixes 
the exact test case I posted.

Unfortunately, my test case was a simplified example of a general 
problem: dso->deps is assigned only for the main app and for libraries 
opened with dlopen(), but not for their dependencies. Consider the 
following:

$ cat bar.c
int bar = 42;
$ musl-gcc -fPIC -shared bar.c -o libbar.so
$ cat foo.c
extern int bar;
int *foo = &bar;
$ cat baz.c
extern int bazdep;
int *baz = &bazdep;
$ cat bazdep.c
int bazdep = 1;
$ cat main.c
#include <dlfcn.h>
#include <stdio.h>

int main(void) {
   if (!dlopen("libbaz.so", RTLD_NOW|RTLD_LOCAL))
     return 1;
   if (!dlopen("libfoo.so", RTLD_NOW|RTLD_LOCAL))
     return 1;
   void *h = dlopen("libbazdep.so", RTLD_NOW|RTLD_LOCAL);
   printf("%p\n", dlsym(h, "bar"));
}
$ musl-gcc main.c -Wl,-rpath='$ORIGIN' -ldl
$ ./a.out
0x7f66ed371020

Here, "libbazdep.so" assumes the role of "libc.so" from the previous 
test: it's a library with dso->deps == NULL that is loaded before 
"libfoo.so". So, when "libbazdep.so" is dlopen'd, musl considers it to 
be a "first load" and erroneously includes "libbar.so" to the list of 
dependencies of "libbazdep.so".

Alexey