Bug 20069 - dlsym() should not use malloc on failure (breaks Address Sanitizer initialization)
Summary: dlsym() should not use malloc on failure (breaks Address Sanitizer initializa...
Status: RESOLVED WORKSFORME
Alias: None
Product: glibc
Classification: Unclassified
Component: dynamic-link (show other bugs)
Version: 2.24
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
: 20474 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-05-10 14:01 UTC by Peter Wu
Modified: 2017-03-10 18:26 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Wu 2016-05-10 14:01:20 UTC
The patch for bug 19509 (dlerror() reporting NULL for dlsym(RTLD_NEXT, ...) errors) introduced a malloc call. This breaks interposing libraries such as ASAN[1] that rely on dlsym(RTLD_NEXT) during initialization (in which malloc might not be usable yet).

 [1]: https://llvm.org/bugs/show_bug.cgi?id=27310

Reproducer:
#define _GNU_SOURCE
#include <stdlib.h>
#include <unistd.h>
#include <dlfcn.h>
#include <string.h>

static void print(const char *str) {
    write(STDOUT_FILENO, str, strlen(str));
}

void *(*malloc_)(size_t size);
__attribute__((constructor)) static void malloc_init(void) {
    malloc_ = dlsym(RTLD_NEXT, "malloc");
}
void *malloc(size_t size) {
    print("malloc called\n");
    return malloc_(size);
}

int main(void) {
    print("main: calling dlsym\n");
    void *p = dlsym(RTLD_NEXT, "non_existing_symbol");
    if (!p) print("dlsym returned NULL\n");
    print("dlerror: ");
    print(dlerror());
    print("\n");
    return 0;
}

Expected results:
main: calling dlsym
dlsym returned NULL
dlerror: malloc called
malloc called
./malloc: undefined symbol: non_existing_symbol

Actual results:
main: calling dlsym
malloc called
dlsym returned NULL
dlerror: malloc called
malloc called
./malloc: undefined symbol: non_existing_symbol

Other considerations:
the dlopen/dlsym manuals do not (dis)allow malloc, but at least the FreeBSD implementation[2] relies on statically allocated memory. For robustness maybe it is not a bad idea to do the same (though this will limit error message lengths).
Comment 1 Florian Weimer 2016-05-10 19:41:33 UTC
We cannot easily use a static buffer because symbol names can be very long indeed, and the buffer would have to be per-thread and pre-allocated (which would be a waste in general).
Comment 2 Sourceware Commits 2016-05-11 09:46:19 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, release/2.23/master has been updated
       via  24e2b1cede1952d7d4411a3cafd25dd8593dab9f (commit)
      from  bbea74b29974d559604691441d12ea80b2abe919 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=24e2b1cede1952d7d4411a3cafd25dd8593dab9f

commit 24e2b1cede1952d7d4411a3cafd25dd8593dab9f
Author: Florian Weimer <fweimer@redhat.com>
Date:   Wed May 11 11:40:44 2016 +0200

    Revert "Report dlsym, dlvsym lookup errors using dlerror [BZ #19509]"
    
    This reverts commits 80f87443eed17838fe453f1f5406ccf5d3698c25
    and a824d609581d5ee7544aabcbbc70e8da44b2b5b6.
    
    See bug 20069.  We can revisit this change once there has been a GCC
    release with a fix for Address Sanitizer.

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog             |   13 ------
 elf/Makefile          |    4 +-
 elf/dl-lookup.c       |    1 +
 elf/tst-dlsym-error.c |  114 -------------------------------------------------
 4 files changed, 2 insertions(+), 130 deletions(-)
 delete mode 100644 elf/tst-dlsym-error.c
Comment 3 Carlos O'Donell 2016-05-11 15:34:55 UTC
We are reverting the breaking change from stable branches, since those won't have time to pickup a coordinated fix. 

I consider this a case where we need to coordinate our resources to schedule a coordinated fix timed with glibc 2.24 (August 1st), gcc (stable branches), and ASAN (stable branches).
Comment 4 Florian Weimer 2016-08-17 15:13:12 UTC
*** Bug 20474 has been marked as a duplicate of this bug. ***
Comment 5 Sourceware Commits 2016-11-12 06:44:08 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, gentoo/2.23 has been updated
       via  5237dbe70306a55036334a4dbf3b7abdff0c8877 (commit)
      from  be3ad4ee1b3481d31741415efc040cca72de26c2 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=5237dbe70306a55036334a4dbf3b7abdff0c8877

commit 5237dbe70306a55036334a4dbf3b7abdff0c8877
Author: Florian Weimer <fweimer@redhat.com>
Date:   Wed May 11 11:40:44 2016 +0200

    Revert "Report dlsym, dlvsym lookup errors using dlerror [BZ #19509]"
    
    This reverts commits 80f87443eed17838fe453f1f5406ccf5d3698c25
    and a824d609581d5ee7544aabcbbc70e8da44b2b5b6.
    
    See bug 20069.  We can revisit this change once there has been a GCC
    release with a fix for Address Sanitizer.
    
    (cherry picked from commit 24e2b1cede1952d7d4411a3cafd25dd8593dab9f)

-----------------------------------------------------------------------

Summary of changes:
 elf/Makefile          |    4 +-
 elf/dl-lookup.c       |    1 +
 elf/tst-dlsym-error.c |  114 -------------------------------------------------
 3 files changed, 2 insertions(+), 117 deletions(-)
 delete mode 100644 elf/tst-dlsym-error.c
Comment 6 Florian Weimer 2017-03-10 18:26:43 UTC
Address Sanitizer has been adjusted, so no further glibc changes are required.