From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.2 required=5.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FROM,MAILING_LIST_MULTI,NICE_REPLY_A, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 1543 invoked from network); 13 Jul 2023 02:03:12 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 13 Jul 2023 02:03:12 -0000 Received: (qmail 22138 invoked by uid 550); 13 Jul 2023 02:03:09 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 22100 invoked from network); 13 Jul 2023 02:03:08 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1689213777; x=1691805777; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=b1RGKPfJvFRzqMD6xXi4uZ9sD2wmM0ZWMVuL9+q0jEs=; b=rVfkbYlaOMhqdgaaADX3s83Oiy4wX96f44+RKqyrh/puNY03q2o9NeNuE7clcBNLJg ph7n+4mLldaKrFUGadVG0or5BpnHj6SQlIRcRI1/WP3EEoGweTv5vEYAmoUDpqDNdzbl Wpz/MkIPzC5MPo6U5O/o3fS2bfjV8AnHcVhGQZhiY3wtY8kBxgQTaexVMpZeXS7DNtAE 2wedFD1Xn5sSxBW8Osc32q+9Pn0JBzznaV200RSrUkqDR9pqTqY6pbB9XjXa+1xh9Qt0 i0POG3uPRChstWnamPcAnYll9fiPcJCB5xmxHYZdwEK94HTA1nhDs6phAdZl/vUyDJPG RsXw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689213777; x=1691805777; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=b1RGKPfJvFRzqMD6xXi4uZ9sD2wmM0ZWMVuL9+q0jEs=; b=ZwQm7YPaDP7egCqj5WI7EATPe6CsUKkwtHJ4r69Vt4Z0KxkDT3fJWj4P5aZCd3rCRf C/3yZE6kKY90kaD21h0+IQqTqA0uFrAa4V54or14s5lzwj0GQHR0iAb/Q8USjVoSh5fO FCUoiXNHnuXaa0eXfcFc04UP1jfkWL8qGWqiPQB74PyOhDVPIG+jFVPMH2kGaESlYpcT jF4KM5rC1GB4ZGYPch5Tsss+vBzNh7y5a8HqJ3DQ6otMLtmqPM7Ip3S9GjimuFd+VrXu vZPrPbIgGXfmHP6Rxgnre35B5LIabEGXz/HCwSdqxsQJX9Zuw4f/5CqVnGWvld0XGdSH Hqtg== X-Gm-Message-State: ABy/qLaMHaaC81+jE89D5ojpVaYBP31UV7GH24SRUhk4UgQ3EZLzoh9P eYZI77GLnhJlothOzEE3Xu8= X-Google-Smtp-Source: APBJJlHggngkmxo1wHphUiqf3yhqvrOcshthZiBJMj0JOi1XLNza9A6RaWjctpuW9nJluAsIzRVKLg== X-Received: by 2002:a05:600c:218d:b0:3fa:91d2:55b6 with SMTP id e13-20020a05600c218d00b003fa91d255b6mr126803wme.9.1689213776852; Wed, 12 Jul 2023 19:02:56 -0700 (PDT) Message-ID: <7f435b07-ee08-8ed1-72a2-edb7354aa2f1@gmail.com> Date: Thu, 13 Jul 2023 04:02:54 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Content-Language: en-US To: Rich Felker Cc: musl@lists.openwall.com, "Appelmans, Madeleine" References: <20230712024804.GH4163@brightrain.aerifal.cx> <20230713015046.GI4163@brightrain.aerifal.cx> From: Gabriel Ravier In-Reply-To: <20230713015046.GI4163@brightrain.aerifal.cx> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [musl] Difference in pthread behavior between glibc and musl 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. >>>> When deleting a key, musl assumes that initialization has been done, >>>> and dereferences tsd without checking that it exists: see >>>> here. >>>> 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.