elf: Never use the file ID of the main executable [BZ #24900]
Commit Message
If the loader is invoked explicitly and loads the main executable,
it stores the file ID of the main executable in l_file_id. This
information is not available if the main excutable is loaded by the
kernel, so this is another case where the two cases differ.
This enhances commit 23d2e5faf0bca6d9b31bef4aa162b95ee64cbfc6
("elf: Self-dlopen failure with explict loader invocation
[BZ #24900]").
2019-08-22 Florian Weimer <fweimer@redhat.com>
[BZ #24900]
* elf/dl-load.c (_dl_map_object_from_fd): Do not use the file ID
when loading the executable as part of an explicit loader
invocation.
Comments
Hi, Florian,
On Thu, 22 Aug 2019, Florian Weimer wrote:
>If the loader is invoked explicitly and loads the main executable,
>it stores the file ID of the main executable in l_file_id. This
>information is not available if the main excutable is loaded by the
>kernel, so this is another case where the two cases differ.
>
>This enhances commit 23d2e5faf0bca6d9b31bef4aa162b95ee64cbfc6
>("elf: Self-dlopen failure with explict loader invocation
>[BZ #24900]").
As we discussed at Cauldron, I had reviewed this patch and noticed that it
changes the place where elf/tst-dlopen-aout fails.
Before this patch, it fails in the test case itself , at:
error: tst-dlopen-aout.c:48: dlopen succeeded unexpectedly: elf/tst-dlopen-aout
After this patch, it fails at:
elf/dl-tls.c: 517: _dl_allocate_tls_init: Assertion `listp != NULL' failed!
which, as you explained to me at Cauldron, is expected and will only be
solved with a sebsequent patch [1]. As you also mentioned to me at
Cauldron, reviewing the second patch is a little tricky, but I'll try to
do it.
[1] https://sourceware.org/ml/libc-alpha/2019-08/msg00625.html
>- /* Get file information. */
>+ /* Get file information. To match the kernel behavior, do not fill
>+ in this information for the executable in case of an explicit
>+ loader invocation. */
> struct r_file_id id;
>+ if (mode & __RTLD_OPENEXEC)
>+ {
>+ assert (nsid == LM_ID_BASE);
>+ memset (&id, 0, sizeof (id));
>+ }
>+ else
>+ {
OK.
I think that this patch does the right thing and could be committed.
Reviewed-by: Gabriel F. T. Gomes <gabrielftg@linux.ibm.com>
Thanks!
@@ -876,33 +876,43 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
struct r_debug *r = _dl_debug_initialize (0, nsid);
bool make_consistent = false;
- /* Get file information. */
+ /* Get file information. To match the kernel behavior, do not fill
+ in this information for the executable in case of an explicit
+ loader invocation. */
struct r_file_id id;
- if (__glibc_unlikely (!_dl_get_file_id (fd, &id)))
+ if (mode & __RTLD_OPENEXEC)
{
- errstring = N_("cannot stat shared object");
- call_lose_errno:
- errval = errno;
- call_lose:
- lose (errval, fd, name, realname, l, errstring,
- make_consistent ? r : NULL, nsid);
+ assert (nsid == LM_ID_BASE);
+ memset (&id, 0, sizeof (id));
}
+ else
+ {
+ if (__glibc_unlikely (!_dl_get_file_id (fd, &id)))
+ {
+ errstring = N_("cannot stat shared object");
+ call_lose_errno:
+ errval = errno;
+ call_lose:
+ lose (errval, fd, name, realname, l, errstring,
+ make_consistent ? r : NULL, nsid);
+ }
- /* Look again to see if the real name matched another already loaded. */
- for (l = GL(dl_ns)[nsid]._ns_loaded; l != NULL; l = l->l_next)
- if (!l->l_removed && _dl_file_id_match_p (&l->l_file_id, &id))
- {
- /* The object is already loaded.
- Just bump its reference count and return it. */
- __close_nocancel (fd);
+ /* Look again to see if the real name matched another already loaded. */
+ for (l = GL(dl_ns)[nsid]._ns_loaded; l != NULL; l = l->l_next)
+ if (!l->l_removed && _dl_file_id_match_p (&l->l_file_id, &id))
+ {
+ /* The object is already loaded.
+ Just bump its reference count and return it. */
+ __close_nocancel (fd);
- /* If the name is not in the list of names for this object add
- it. */
- free (realname);
- add_name_to_object (l, name);
+ /* If the name is not in the list of names for this object add
+ it. */
+ free (realname);
+ add_name_to_object (l, name);
- return l;
- }
+ return l;
+ }
+ }
#ifdef SHARED
/* When loading into a namespace other than the base one we must