[Bug libc/11952] New: glibc may use uninitialized DTV slot, return NULL for &thread_local

[Bug libc/11952] New: glibc may use uninitialized DTV slot, return NULL for &thread_local

[ppluzhnikov at google dot com]

When a shared library foo.so with TLS variables is loaded as a result
of dlopen("bar.so"), and bar.so has R_386_TLS_TPOFF, then glibc may
use unitialized (zero'd) dtv[] entry, and address of thread-local variable
will become NULL.

The bug exists in glibc-2.7 and current git HEAD.

Repro instructions:

cat > foo.c <<EOF
static __thread int foo;
int *get_foo() { return &foo; }
EOF

cat > bar.c <<EOF
static __thread int bar;
extern int *get_bar() { return &bar; }
EOF

cat > main.c <<EOF
#include <stdio.h>
#include <dlfcn.h>

int main()
{
  void *h = dlopen("./bar.so", RTLD_LAZY);
  int* (*get_bar)(void) = dlsym(h, "get_bar");
  int* (*get_foo)(void) = dlsym(h, "get_foo");

  printf("get_bar() = %p\n", (*get_bar)());
  printf("get_foo() = %p\n", (*get_foo)());

  return 0;
}
EOF

gcc -m32 -fPIC -shared -o foo.so foo.c
gcc -m32 -shared -o bar.so bar.c ./foo.so  # NOTE: no -fPIC
gcc -m32 main.c -ldl

./a.out
get_bar() = 0xf756e680
get_foo() = (nil)          <<<- BUG


Rebuilding bar.so with -fPIC makes the problem go away:

gcc -m32 -fPIC -shared -o bar.so bar.c ./foo.so
./a.out
get_bar() = 0x804a700
get_foo() = 0x804a710

-- 
           Summary: glibc may use uninitialized DTV slot, return NULL for
                    &thread_local
           Product: glibc
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: libc
        AssignedTo: drepper at redhat dot com
        ReportedBy: ppluzhnikov at google dot com
                CC: glibc-bugs at sources dot redhat dot com
 GCC build triplet: x86_64-unknown-linux-gnu
  GCC host triplet: x86_64-unknown-linux-gnu
GCC target triplet: i586-unknown-linux-gnu


http://sourceware.org/bugzilla/show_bug.cgi?id=11952

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/11952] glibc may use uninitialized DTV slot, return NULL for &thread_local

[drepper at gmail dot com]

------- Additional Comments From drepper at gmail dot com  2010-09-16 00:13 -------
(In reply to comment #1)
> Created an attachment (id=4957)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=4957&action=view)
> Proposed patch that fixes the problem

This might or might not be the right way to handle this.  What bothers me most, 
though, is that this code seems to be unnecessary on sane platforms which don't 
allow incorrect code like non-PIC objects in DSOs.

I have to think a lot more about this.



-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=11952

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/11952] glibc may use uninitialized DTV slot, return NULL for &thread_local

[ppluzhnikov at google dot com]

------- Additional Comments From ppluzhnikov at google dot com  2010-09-16 16:30 -------
(In reply to comment #2)
> this code seems to be unnecessary on sane platforms which don't 
> allow incorrect code like non-PIC objects in DSOs.

True, but the insane platform is here to stay :-(

The code is already going to a lot of trouble to support the insane platform, 
and the patch doesn't add anything to that work; it just slightly rearranges the 
order in which things are done.

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=11952

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/11952] glibc may use uninitialized DTV slot, return NULL for &thread_local

[drepper at gmail dot com]

------- Additional Comments From drepper at gmail dot com  2010-09-16 19:44 -------
(In reply to comment #3)
> True, but the insane platform is here to stay :-(

Unfortunately, but we don't have to allow non-PIC in DSOs.  In fact, don't 
recent binutils versions prevent this?  That's definitely something that should 
be done.


> The code is already going to a lot of trouble to support the insane platform, 
> and the patch doesn't add anything to that work; it just slightly rearranges
> the order in which things are done.

You seem not to understand the costs.  Running over a list twice with two short 
loops is much slower than running over them once with a combined loop body.



-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=11952

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/11952] glibc may use uninitialized DTV slot, return NULL for &thread_local

[ppluzhnikov at google dot com]

------- Additional Comments From ppluzhnikov at google dot com  2010-09-16 20:39 -------
(In reply to comment #4)

> Running over a list twice with two short 
> loops is much slower than running over them once with a combined loop body.

After the patch, the second loop executes only until we've accounted for all
"broken" (i.e. non-PIC but using TLS) DSOs.

So in the usual (non-broken) case tls_counter == 0 and there is no additional
cost (that I can see).

In the broken case,
A) who cares -- the user should just use PIC; and 
B) we stop the second loop as soon as we can (when tls_counter reaches 0), which
can still be the entire list in the worst case, but likely much sooner in practice.

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=11952

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/11952] glibc may use uninitialized DTV slot, return NULL for &thread_local

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=11952

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |security-

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/11952] glibc may use uninitialized DTV slot, return NULL for &thread_local

[drepper.fsp at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=11952

Ulrich Drepper <drepper.fsp at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |DUPLICATE

--- Comment #6 from Ulrich Drepper <drepper.fsp at gmail dot com> 2011-05-15 01:33:07 UTC ---
It also effects correct code as in 12453.

*** This bug has been marked as a duplicate of bug 12453 ***

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.