This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] elf: Refuse to dlopen PIE objects [BZ #24323]
- From: Carlos O'Donell <codonell at redhat dot com>
- To: Florian Weimer <fweimer at redhat dot com>, libc-alpha at sourceware dot org
- Date: Mon, 29 Apr 2019 17:18:53 -0400
- Subject: Re: [PATCH] elf: Refuse to dlopen PIE objects [BZ #24323]
- References: <20190312130235.8E82C89CE49C@oldenburg2.str.redhat.com>
On 3/12/19 9:02 AM, Florian Weimer wrote:
Another executable has already been mapped, so the dynamic linker
cannot perform relocations correctly for the second executable.
2019-03-12 Florian Weimer <fweimer@redhat.com>
[BZ #24323]
* include/elf.h (DT_1_SUPPORTED_MASK): Include DF_1_PIE.
* elf/dl-load.c (_dl_map_object_from_fd): Check for DF_1_PIE and
fail when called from dlopen.
* elf/Makefile [have-fpie && build-shared] (tests): Add
tst-dlopen-pie.
(tst-dlopen-pie): Link with -ldl.
(tst-dlopen-pie.out): Add run-time dependency on tst-pie1.
* elf/tst-dlopen-pie.c (do_test): New file.
This makes perfect sense.
OK if you expand the comment below.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
diff --git a/elf/Makefile b/elf/Makefile
index 310a37cc13..ef861abac8 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -305,7 +305,7 @@ test-xfail-tst-protected1b = yes
endif
ifeq (yesyes,$(have-fpie)$(build-shared))
modules-names += tst-piemod1
-tests += tst-pie1 tst-pie2
+tests += tst-pie1 tst-pie2 tst-dlopen-pie
OK.
tests-pie += tst-pie1 tst-pie2
ifeq (yes,$(have-protected-data))
tests += vismain
@@ -1079,6 +1079,8 @@ CFLAGS-tst-pie2.c += $(pie-ccflag)
$(objpfx)tst-piemod1.so: $(libsupport)
$(objpfx)tst-pie1: $(objpfx)tst-piemod1.so
+$(objpfx)tst-dlopen-pie: $(libdl)
+$(objpfx)tst-dlopen-pie.out: $(objpfx)tst-pie1
OK.
ifeq (yes,$(build-shared))
# NB: Please keep cet-built-dso in sysdeps/x86/Makefile in sync with
diff --git a/elf/dl-load.c b/elf/dl-load.c
index 2bbef81b6e..9b0c1406c9 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -1158,6 +1158,10 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
goto call_lose;
}
+ /* dlopen of an executable is not valid because it is not possible
+ to perform proper relocations and run the ELF constructors.
+ For PIE, the check needs the dynamic section, so there is
+ another check below. */
Suggest you mention exactly which relocations are the problem.
I assume it's COPY relocs into the main executable which can't be
done twice, because both execs will want to reference their own
copy. Likewise the problem with ELF constructors should be expanded
with more details.
if (__glibc_unlikely (type != ET_DYN)
&& __glibc_unlikely ((mode & __RTLD_OPENEXEC) == 0))
{
@@ -1194,9 +1198,11 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
elf_get_dynamic_info (l, NULL);
/* Make sure we are not dlopen'ing an object that has the
- DF_1_NOOPEN flag set. */
- if (__glibc_unlikely (l->l_flags_1 & DF_1_NOOPEN)
- && (mode & __RTLD_DLOPEN))
+ DF_1_NOOPEN flag set, or a PIE object. */
+ if ((__glibc_unlikely (l->l_flags_1 & DF_1_NOOPEN)
+ && (mode & __RTLD_DLOPEN))
+ || (__glibc_unlikely (l->l_flags_1 & DF_1_PIE)
+ && __glibc_unlikely ((mode & __RTLD_OPENEXEC) == 0)))
OK.
{
/* We are not supposed to load this object. Free all resources. */
_dl_unmap_segments (l);
@@ -1207,7 +1213,11 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
if (l->l_phdr_allocated)
free ((void *) l->l_phdr);
- errstring = N_("shared object cannot be dlopen()ed");
+ if (l->l_flags_1 & DF_1_PIE)
+ errstring
+ = N_("cannot dynamically load position-independent executable");
OK.
+ else
+ errstring = N_("shared object cannot be dlopen()ed");
goto call_lose;
}
diff --git a/elf/tst-dlopen-pie.c b/elf/tst-dlopen-pie.c
new file mode 100644
index 0000000000..6a41c731cb
--- /dev/null
+++ b/elf/tst-dlopen-pie.c
@@ -0,0 +1,49 @@
+/* dlopen test for PIE objects.
+ Copyright (C) 2019 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
+ <http://www.gnu.org/licenses/>. */
+
+/* This test attempts to open the (otherwise unrelated) PIE test
+ program elf/tst-pie1 and expects the attempt to fail. */
OK.
+
+#include <dlfcn.h>
+#include <stddef.h>
+#include <string.h>
+#include <support/check.h>
+#include <support/support.h>
+
+static void
+test_mode (int mode)
+{
+ char *pie_path = xasprintf ("%s/elf/tst-pie1", support_objdir_root);
+ if (dlopen (pie_path, mode) != NULL)
OK.
+ FAIL_EXIT1 ("dlopen succeeded unexpectedly (%d)", mode);
+ const char *message = dlerror ();
+ const char *expected
+ = "cannot dynamically load position-independent executable";
+ if (strstr (message, expected) == NULL)
+ FAIL_EXIT1 ("unexpected error message (mode %d): %s", mode, message);
+}
+
+static int
+do_test (void)
+{
+ test_mode (RTLD_LAZY);
+ test_mode (RTLD_NOW);
OK, run in both modes for coverage.
+ return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/include/elf.h b/include/elf.h
index ab76aafb1e..14ed67ff67 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -23,7 +23,7 @@
# endif
# define DT_1_SUPPORTED_MASK \
(DF_1_NOW | DF_1_NODELETE | DF_1_INITFIRST | DF_1_NOOPEN \
- | DF_1_ORIGIN | DF_1_NODEFLIB)
+ | DF_1_ORIGIN | DF_1_NODEFLIB | DF_1_PIE)
OK.
#endif /* !_ISOMAC */
#endif /* elf.h */
--
Cheers,
Carlos.