Bug 26955 - pthread_key_create may return duplicates if libpthread.so loaded more than once
Summary: pthread_key_create may return duplicates if libpthread.so loaded more than once
Status: RESOLVED DUPLICATE of bug 24776
Alias: None
Product: glibc
Classification: Unclassified
Component: nptl (show other bugs)
Version: 2.34
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-11-26 18:30 UTC by Reimar Döffinger
Modified: 2021-05-31 18:10 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
Test-case using both dlopen and dlmopen (637 bytes, text/x-csrc)
2020-11-26 18:30 UTC, Reimar Döffinger
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Reimar Döffinger 2020-11-26 18:30:26 UTC
Created attachment 13000 [details]
Test-case using both dlopen and dlmopen

pthread_key_create directly accesses a global variable (__pthread_keys) to scan for a free TSS location.
This means if the library is loaded multiple times and there a multiple instances then there will be duplicates, leading to corruption.

This can happen for example if there is a libpthread with a different name or if dlmopen is used.

The error here is that the thread local storage allocation data structure is associated with a specific libpthread instead of with the thread, which could be fixed by putting a pointer to __pthread_keys into THREAD_SELF and using that instead (alternatively, it could be partially mitigated if the values were stored in per-library storage by pthread_setspecific, but that seems harder and weirder).
This by itself would avoid the corruption within a single thread/in single threaded programs at least, however if new threads are created using pthread_create from different libpthread instances the keys would no longer be unique across the whole process.
I believe this remaining issue could be avoided by also copying the pointer in pthread_create (instead of setting it to the "current" libpthread's __pthread_keys), thus distributing a pointer to the same __pthread_keys to all threads no matter which library created them. However for that I have not checked the code.

Note that none of this will prevent things failing completely if incompatible versions of libpthread.so are combined, but that seems reasonable to leave out of scope (and is likely to fail in all kinds of other ways).

Also see attached test-case. For the dlopen part to work you need to copy your system's libpthread.so.0 to libpthread_copy.so.0 in the run directory.
The bug can be seen by the value stored in keys[0] being overwritten using completely different keys.

This issue is the cause of e.g. https://stackoverflow.com/questions/55008283/dlmopen-and-c-libraries/65026781#65026781 (which means that basically anything using libglib breaks very quickly when using it with dlmopen). A test-case for that one is at https://github.com/mhier/segregatedLinkingExample
By the comments bug 24776 seems to be a different issue, but I do not understand all of the discussion, so possibly it is a duplicate at least in part.

Note: I personally have not encountered this issue, but I have a use-case for dlmopen that I need to understand the risks of. This issue getting fixed would make it much less worrisome to rely on dlmopen.
Comment 1 Carlos O'Donell 2020-11-26 19:56:54 UTC
I don't think there is anything wrong with the observed results, for RTLD_LOCAL you get what the API said it does, and for dlmopen we could do better, but what is there today is an expression of the current semantics.

There are two issues:

(a) Using RTLD_LOCAL to load a distinct copy of the same library.
- You have reloaded a copy of the implementation into it's own scope.
- There is no guarantee that this implementation needs to coordinate with the currently loaded version.
- There is no guarantee that they are even compatible.
- I don't think this use case is particularly relevant.
- If you think this is relevant, please expand on this.

(b) Using dlmopen to load a distinct copy of the same library.
- The purpose of dlmopen is exactly to isolate libraries and so a dlmopen version of libpthread should not interact with anything else and that means some APIs will be decoupled in this way.
- In the most recent discussions around improving dlmopen we have discussed two modes of operation:
(b.1) Isolation from the "C implementation" and up.
-- This is what the stackoverflow poster wants, but isn't implemented (until we fix and commit Vivek's patches).
(b.2) Complete isolation.
-- This is what is implemented today, and it's dangerous to take anything but very simple objects across the dlmopen boundary.
- In the case of (b.1) we extend the C implementation into every namespace created by dlmopen and provide consistent APIs for the subsequently loaded libraries. Every namespace though would start with the dynamic loader and the C library (which will soon include libpthread as we migrate everything to a single library).
- In the case of (b.2), and audit modules want this, you don't proxy any global objects into the namespace, you get full isolation, but once you enter the namespace you never exit it (audit modules are always run in their own namespace and can't share data outside of them), and it's unsafe to share across the namespace boundaries.

In summary:
- Using RTLD_LOCAL to load a second copy of the C library that works but doesn't quite coordinate with the first C library is not an interesting problem to solve.
- Using dlmopen and having coordination issues is something that would be interesting to solve, and we have some patches for that:
https://sourceware.org/legacy-ml/libc-alpha/2019-08/msg00442.html

Does that give you some direction?
Comment 2 Florian Weimer 2021-05-31 18:10:44 UTC
Let's track this under the older, related bug.

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