* [musl] Difference in pthread behavior between glibc and musl @ 2023-07-11 19:19 Appelmans, Madeleine 2023-07-11 19:42 ` Markus Wichmann 2023-07-12 2:48 ` Rich Felker 0 siblings, 2 replies; 8+ messages in thread From: Appelmans, Madeleine @ 2023-07-11 19:19 UTC (permalink / raw) To: musl [-- Attachment #1.1: Type: text/plain, Size: 818 bytes --] Hello, There seems to be a difference in pthread behavior when compiling with glibc and using the musl-gcc wrapper. The attached snippet of code creates a destructor attribute which deletes a pthread key. The code never actually creates the pthread key. This code segfaults when compiled using musl-gcc, and does not segfault when compiled with gcc. Best guess at what is going on: When creating a pthread key, musl initializes a field called tsd<https://git.musl-libc.org/cgit/musl/tree/src/thread/pthread_key_create.c#n37>. When deleting a key, musl assumes that initialization has been done, and dereferences tsd without checking that it exists: see here<https://git.musl-libc.org/cgit/musl/tree/src/thread/pthread_key_create.c#n65>. This dereference may be the source of the segfault. Thanks, Madeleine [-- Attachment #1.2: Type: text/html, Size: 2894 bytes --] [-- Attachment #2: pthread_test.c --] [-- Type: application/octet-stream, Size: 217 bytes --] #include <pthread.h> static pthread_key_t per_thread_key; static void __attribute__((destructor)) key_cleanup(void) { pthread_key_delete(per_thread_key); } int main(int argc, char **argv) { // Do nothing } ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] Difference in pthread behavior between glibc and musl 2023-07-11 19:19 [musl] Difference in pthread behavior between glibc and musl Appelmans, Madeleine @ 2023-07-11 19:42 ` Markus Wichmann 2023-07-12 2:48 ` Rich Felker 1 sibling, 0 replies; 8+ messages in thread From: Markus Wichmann @ 2023-07-11 19:42 UTC (permalink / raw) To: musl Am Tue, Jul 11, 2023 at 07:19:50PM +0000 schrieb Appelmans, Madeleine: > Hello, > > There seems to be a difference in pthread behavior when compiling with > glibc and using the musl-gcc wrapper. The attached snippet of code > creates a destructor attribute which deletes a pthread key. The code > never actually creates the pthread key. This code segfaults when > compiled using musl-gcc, and does not segfault when compiled with gcc. > Undefined behavior is undefined. And it is undefined behavior to be calling pthread_key_delete() with a key that was not previously acquired from pthread_key_create(). POSIX encourages - but does not require - implementations to detect invalid keys and return an error then. And musl doesn't. Ciao, Markus ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] Difference in pthread behavior between glibc and musl 2023-07-11 19:19 [musl] Difference in pthread behavior between glibc and musl Appelmans, Madeleine 2023-07-11 19:42 ` Markus Wichmann @ 2023-07-12 2:48 ` Rich Felker 2023-07-13 1:00 ` Gabriel Ravier 1 sibling, 1 reply; 8+ messages in thread From: Rich Felker @ 2023-07-12 2:48 UTC (permalink / raw) To: Appelmans, Madeleine; +Cc: musl On Tue, Jul 11, 2023 at 07:19:50PM +0000, Appelmans, Madeleine wrote: > Hello, > > There seems to be a difference in pthread behavior when compiling > with glibc and using the musl-gcc wrapper. The attached snippet of > code creates a destructor attribute which deletes a pthread key. The > code never actually creates the pthread key. This code segfaults > when compiled using musl-gcc, and does not segfault when compiled > with gcc. > > Best guess at what is going on: When creating a pthread key, musl > initializes a field called > tsd<https://git.musl-libc.org/cgit/musl/tree/src/thread/pthread_key_create.c#n37>. > When deleting a key, musl assumes that initialization has been done, > and dereferences tsd without checking that it exists: see > here<https://git.musl-libc.org/cgit/musl/tree/src/thread/pthread_key_create.c#n65>. > This dereference may be the source of the segfault. This is completely expected; the behavior is undefined because you passed to pthread_key_delete a value which was not acquired via pthread_key_create. If it happens not to crash on glibc, that doesn't mean it's okay to do it. It will end up deleting whatever key happens to correspond to the zero-initialized pthread_key_t object, which may be a key that was allocated for use by some other part of the program when it called pthread_key_create. (In other words, you have a type of double-free or use-after-free bug.) Your program logic must ensure you refrain from doing that. Rich ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] Difference in pthread behavior between glibc and musl 2023-07-12 2:48 ` Rich Felker @ 2023-07-13 1:00 ` Gabriel Ravier 2023-07-13 1:50 ` Rich Felker 0 siblings, 1 reply; 8+ messages in thread From: Gabriel Ravier @ 2023-07-13 1:00 UTC (permalink / raw) To: musl, Rich Felker, Appelmans, Madeleine On 7/12/23 04:48, Rich Felker wrote: > On Tue, Jul 11, 2023 at 07:19:50PM +0000, Appelmans, Madeleine wrote: >> Hello, >> >> There seems to be a difference in pthread behavior when compiling >> with glibc and using the musl-gcc wrapper. The attached snippet of >> code creates a destructor attribute which deletes a pthread key. The >> code never actually creates the pthread key. This code segfaults >> when compiled using musl-gcc, and does not segfault when compiled >> with gcc. >> >> Best guess at what is going on: When creating a pthread key, musl >> initializes a field called >> tsd<https://git.musl-libc.org/cgit/musl/tree/src/thread/pthread_key_create.c#n37>. >> When deleting a key, musl assumes that initialization has been done, >> and dereferences tsd without checking that it exists: see >> here<https://git.musl-libc.org/cgit/musl/tree/src/thread/pthread_key_create.c#n65>. >> This dereference may be the source of the segfault. > This is completely expected; the behavior is undefined because you > passed to pthread_key_delete a value which was not acquired via > pthread_key_create. > > If it happens not to crash on glibc, that doesn't mean it's okay to do > it. It will end up deleting whatever key happens to correspond to the > zero-initialized pthread_key_t object, which may be a key that was > allocated for use by some other part of the program when it called > pthread_key_create. (In other words, you have a type of double-free or > use-after-free bug.) Your program logic must ensure you refrain from > doing that. > > Rich Hmmm, this does indeed seem to be the case ever since SUSv4/XPG7/POSIX.1-2008 removed the EINVAL error from the specification of pthread_key_delete, but the requirement to detect this error is present in SUSv3/XPG6/POSIX.1-2001 (up to and including the 2004 edition). so if you want musl to be able to say it conforms to that standard, it would have to implement this, which after a quick look at musl's source code w.r.t. pthread keys doesn't seem particularly burdensome. Personally I'd be in favor of having this detection occur regardless of whether musl aims for conformance to older standards given that POSIX still now explicitly recommends it even in the standards where it is not mandatory anymore, which is something I've usually seen done in cases where they wanted to specify a behavior but had to back down on making it mandatory on the basis of certain implementations complaining that it was too complicated to implement and/or wanting to retain another behavior for backwards compatibility or other stuff like that - i.e. they would very much prefer implementations where it isn't particularly burdensome to implement it to do so. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] Difference in pthread behavior between glibc and musl 2023-07-13 1:00 ` Gabriel Ravier @ 2023-07-13 1:50 ` Rich Felker 2023-07-13 2:02 ` Gabriel Ravier 0 siblings, 1 reply; 8+ messages in thread From: Rich Felker @ 2023-07-13 1:50 UTC (permalink / raw) To: Gabriel Ravier; +Cc: musl, Appelmans, Madeleine On Thu, Jul 13, 2023 at 03:00:55AM +0200, Gabriel Ravier wrote: > On 7/12/23 04:48, Rich Felker wrote: > >On Tue, Jul 11, 2023 at 07:19:50PM +0000, Appelmans, Madeleine wrote: > >>Hello, > >> > >>There seems to be a difference in pthread behavior when compiling > >>with glibc and using the musl-gcc wrapper. The attached snippet of > >>code creates a destructor attribute which deletes a pthread key. The > >>code never actually creates the pthread key. This code segfaults > >>when compiled using musl-gcc, and does not segfault when compiled > >>with gcc. > >> > >>Best guess at what is going on: When creating a pthread key, musl > >>initializes a field called > >>tsd<https://git.musl-libc.org/cgit/musl/tree/src/thread/pthread_key_create.c#n37>. > >>When deleting a key, musl assumes that initialization has been done, > >>and dereferences tsd without checking that it exists: see > >>here<https://git.musl-libc.org/cgit/musl/tree/src/thread/pthread_key_create.c#n65>. > >>This dereference may be the source of the segfault. > >This is completely expected; the behavior is undefined because you > >passed to pthread_key_delete a value which was not acquired via > >pthread_key_create. > > > >If it happens not to crash on glibc, that doesn't mean it's okay to do > >it. It will end up deleting whatever key happens to correspond to the > >zero-initialized pthread_key_t object, which may be a key that was > >allocated for use by some other part of the program when it called > >pthread_key_create. (In other words, you have a type of double-free or > >use-after-free bug.) Your program logic must ensure you refrain from > >doing that. > > > >Rich > > Hmmm, this does indeed seem to be the case ever since > SUSv4/XPG7/POSIX.1-2008 removed the EINVAL error from the > specification of pthread_key_delete, but the requirement to detect > this error is present in SUSv3/XPG6/POSIX.1-2001 (up to and > including the 2004 edition). so if you want musl to be able to say > it conforms to that standard, it would have to implement this, which > after a quick look at musl's source code w.r.t. pthread keys doesn't > seem particularly burdensome. There was never a requirement to detect UAF/DF type errors. There was a requirement ("shall fail"), *if the implementation does detect it*, to fail with EINVAL. This was nonsensical because there was no requirement to do the detection (and in general the detection is impossible; that's the nature of UAF/DF) so the confusing text was fixed. But there was no change in the actual requirement. > Personally I'd be in favor of having this detection occur regardless > of whether musl aims for conformance to older standards given that > POSIX still now explicitly recommends it even in the standards where > it is not mandatory anymore, which is something I've usually seen It is not "explicitly recommended", and it's against best practices to return an error rather than trapping on UB. Rich ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] Difference in pthread behavior between glibc and musl 2023-07-13 1:50 ` Rich Felker @ 2023-07-13 2:02 ` Gabriel Ravier 2023-07-13 16:27 ` Markus Wichmann 0 siblings, 1 reply; 8+ messages in thread From: Gabriel Ravier @ 2023-07-13 2:02 UTC (permalink / raw) To: Rich Felker; +Cc: musl, Appelmans, Madeleine On 7/13/23 03:50, Rich Felker wrote: > On Thu, Jul 13, 2023 at 03:00:55AM +0200, Gabriel Ravier wrote: >> On 7/12/23 04:48, Rich Felker wrote: >>> On Tue, Jul 11, 2023 at 07:19:50PM +0000, Appelmans, Madeleine wrote: >>>> Hello, >>>> >>>> There seems to be a difference in pthread behavior when compiling >>>> with glibc and using the musl-gcc wrapper. The attached snippet of >>>> code creates a destructor attribute which deletes a pthread key. The >>>> code never actually creates the pthread key. This code segfaults >>>> when compiled using musl-gcc, and does not segfault when compiled >>>> with gcc. >>>> >>>> Best guess at what is going on: When creating a pthread key, musl >>>> initializes a field called >>>> tsd<https://git.musl-libc.org/cgit/musl/tree/src/thread/pthread_key_create.c#n37>. >>>> When deleting a key, musl assumes that initialization has been done, >>>> and dereferences tsd without checking that it exists: see >>>> here<https://git.musl-libc.org/cgit/musl/tree/src/thread/pthread_key_create.c#n65>. >>>> This dereference may be the source of the segfault. >>> This is completely expected; the behavior is undefined because you >>> passed to pthread_key_delete a value which was not acquired via >>> pthread_key_create. >>> >>> If it happens not to crash on glibc, that doesn't mean it's okay to do >>> it. It will end up deleting whatever key happens to correspond to the >>> zero-initialized pthread_key_t object, which may be a key that was >>> allocated for use by some other part of the program when it called >>> pthread_key_create. (In other words, you have a type of double-free or >>> use-after-free bug.) Your program logic must ensure you refrain from >>> doing that. >>> >>> Rich >> Hmmm, this does indeed seem to be the case ever since >> SUSv4/XPG7/POSIX.1-2008 removed the EINVAL error from the >> specification of pthread_key_delete, but the requirement to detect >> this error is present in SUSv3/XPG6/POSIX.1-2001 (up to and >> including the 2004 edition). so if you want musl to be able to say >> it conforms to that standard, it would have to implement this, which >> after a quick look at musl's source code w.r.t. pthread keys doesn't >> seem particularly burdensome. > There was never a requirement to detect UAF/DF type errors. There was > a requirement ("shall fail"), *if the implementation does detect it*, > to fail with EINVAL. This was nonsensical because there was no > requirement to do the detection (and in general the detection is > impossible; that's the nature of UAF/DF) so the confusing text was > fixed. But there was no change in the actual requirement. > >> Personally I'd be in favor of having this detection occur regardless >> of whether musl aims for conformance to older standards given that >> POSIX still now explicitly recommends it even in the standards where >> it is not mandatory anymore, which is something I've usually seen > It is not "explicitly recommended", and it's against best practices to > return an error rather than trapping on UB. > > Rich Well, the standard states that: > If an implementation detects that the value specified by the key argument to pthread_key_delete( ) does not refer to a a key value obtained from pthread_key_create( ) or refers to a key that has been deleted with pthread_key_delete( ), it is recommended that the function should fail and report an [EINVAL] error. so I don't get you mean by 'It is not "explicitly recommended"': I can't imagine any statement that would make it more clear it is explicitly recommended. Though, on the other hand, I generally agree that it's not a great idea to just return an error on invalid keys instead of crashing, even though doing so is consistent with e.g. EBADF being returned for invalid file descriptors in system calls, even though an invalid fd pretty much systematically indicates a grave bug in a program. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] Difference in pthread behavior between glibc and musl 2023-07-13 2:02 ` Gabriel Ravier @ 2023-07-13 16:27 ` Markus Wichmann 2023-07-13 18:49 ` Gabriel Ravier 0 siblings, 1 reply; 8+ messages in thread From: Markus Wichmann @ 2023-07-13 16:27 UTC (permalink / raw) To: musl Am Thu, Jul 13, 2023 at 04:02:54AM +0200 schrieb Gabriel Ravier: > so I don't get you mean by 'It is not "explicitly recommended"': Reading comprehension, please! The sentence you quoted contains a condition; one that Rich already pointed out to you. The condition is not fulfilled; musl does not detect that the key is invalid, and therefore the entire thing is null for this implementation. Let alone that the sentence is in a section explicitly marked "informative" (as opposed to normative). Ciao, Markus ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] Difference in pthread behavior between glibc and musl 2023-07-13 16:27 ` Markus Wichmann @ 2023-07-13 18:49 ` Gabriel Ravier 0 siblings, 0 replies; 8+ messages in thread From: Gabriel Ravier @ 2023-07-13 18:49 UTC (permalink / raw) To: musl, Markus Wichmann On 7/13/23 18:27, Markus Wichmann wrote: > Am Thu, Jul 13, 2023 at 04:02:54AM +0200 schrieb Gabriel Ravier: >> so I don't get you mean by 'It is not "explicitly recommended"': > Reading comprehension, please! The sentence you quoted contains a > condition; one that Rich already pointed out to you. The condition is > not fulfilled; musl does not detect that the key is invalid, and > therefore the entire thing is null for this implementation. Let alone > that the sentence is in a section explicitly marked "informative" (as > opposed to normative). > > Ciao, > Markus I know it's not normative - that doesn't seem like it would change anything given that regardless is a recommendation, not a requirement. The explanation you've given does makes sense though - the wording seems oddly ambiguous, but that does seem to be what it's trying to say. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-07-13 18:49 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-07-11 19:19 [musl] Difference in pthread behavior between glibc and musl Appelmans, Madeleine 2023-07-11 19:42 ` Markus Wichmann 2023-07-12 2:48 ` Rich Felker 2023-07-13 1:00 ` Gabriel Ravier 2023-07-13 1:50 ` Rich Felker 2023-07-13 2:02 ` Gabriel Ravier 2023-07-13 16:27 ` Markus Wichmann 2023-07-13 18:49 ` Gabriel Ravier
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/musl/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).