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=-2.2 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FORGED_HOTMAIL_RCVD2,FREEMAIL_FROM,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 32261 invoked from network); 6 Oct 2021 14:25:07 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 6 Oct 2021 14:25:07 -0000 Received: (qmail 28081 invoked by uid 550); 6 Oct 2021 14:25:05 -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 9700 invoked from network); 6 Oct 2021 12:58:10 -0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ljMk2IMTaSQuo3ZBwdFkfQ5bi7iJd66peRyyZ/LQsx5wn6spykvLwOL1IlOXqq/nSn/d/1f2IVFotu4qQwIo6TvrB2NRkf2GFMpFWf5Y8Y9WcEE4J0BplVDV3z+1jfb15zmP+md9rPn59xAbvPHVVPwKoqUI3Pzj+JSgMlCGc1CL4V/3mHhTDiJFNhfkl05JNcuNLTJ8M1T0JTa7kPami+7e1W4zA4O+ozZwgTkAkFrWPuPjEN97L4YLq3g7/M7gjKtFxPNQvo3x8/NktQ6kCAB3mPhgJByjvVDNoLG2wcs9QRuYsxorYEXooC6qAB5TE7yitaUI0zyJTQ2pjnnO+Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=nc84v9V0ApG0tM0MOKL+ca1esKeqSq+zijmWFmFr6DM=; b=Pc8hm03rJjgTiWhOgF6yP1hsCE07mmpXetncQ7ATXv0Ck9xeNjdsphcqitswaxfqHlgaoWhCNk3RG0RIwzw3HTyPv7jM13GoBCwP6cPkn1m/Rf15N5BuTCrXWUoMgoLmgUyCyL+63GNJ7Xj8JS23NTli6q3SYWX9/tnlEnivsKjNtJcOKl0P6+tKKRcHuTOig75SJys1atY8AIjo/kjuGkhqtZaUV9QgSo/NdAVbz+AmIesvuWBSEMPK+3l8MH9a2jiDwMGxXE4uky1VJ8NlDTK8EQ2dtirUyejZjkoW+7dYsvn50yHsQwoV0MvH0LseECJgMoLuGjkfIddZvwamgg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=hotmail.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=nc84v9V0ApG0tM0MOKL+ca1esKeqSq+zijmWFmFr6DM=; b=iWVp2TYvRhs6pyOcHTtLwIGc/HMEz5jId7dQjvKELdkDV7solk8wgm5gM/JC34t7YVfK91PTbToiVpd4hprBS13tnvGG8HySOLEelgiMS+g2wk8Z3lmMD4AW1ZA1+DEydpi8e3F4tI0s1eJ+vdbiwBUSIznsCoBZoy+nS0CBFb7eRRvXUz2ogyjyP8Eo2rV7Can9CphMA0M/7tJYcos5u47994aR3k8QV/b0rNkckLm0cuXIcsigFItCO6lBfaBkFyQE81EN7P4cQMqkb5JIq7Y+JKUSKEEz0NV++MuBlf/myTG9XjIVZK9pTkWQPcQnQUIj4fH37ycKgrOJ6UdgwQ== X-IncomingTopHeaderMarker: OriginalChecksum:07540D1E0B6B6583BD50877DA463FEAFA8D1C81E309A2D7ECA08A14F73469627;UpperCasedChecksum:D581D83989CA7567FEF96D3A0E7B5DEACF63316CFC7F0AC68EE4B45D8F9861B5;SizeAsReceived:8617;Count:47 Message-ID: From: Pablo Correa Gomez To: Rich Felker Cc: musl@lists.openwall.com Date: Wed, 06 Oct 2021 14:57:55 +0200 In-Reply-To: <20211006122936.GD2559@brightrain.aerifal.cx> References: <20211006122936.GD2559@brightrain.aerifal.cx> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.36.5-0ubuntu1 Content-Transfer-Encoding: 7bit X-TMN: [jPbVA0G4Y/gow1MKZ1i9yeggQRWjsCz5] X-ClientProxiedBy: MR1P264CA0014.FRAP264.PROD.OUTLOOK.COM (2603:10a6:501:2e::19) To AM5P192MB0081.EURP192.PROD.OUTLOOK.COM (2603:10a6:203:81::10) X-Microsoft-Original-Message-ID: <7e09daee5fe424ca84b636115c2b330a93308ce2.camel@hotmail.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-IncomingHeaderCount: 47 X-EOPAttributedMessage: 0 X-MS-Office365-Filtering-Correlation-Id: c8a30892-2e66-47e4-cb27-08d988c8eb6e X-MS-TrafficTypeDiagnostic: AM7EUR06HT053: X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 5haFH5PMlxd0W7eyuN9lXKAGZY/rbqD2m+8oOk6T6t0S5yOzW6VzlkVA3Hy8RHyoP+9YH7PWRA37t/Mu6+fB4p/0tdUseMGbiq1JFAEzioMa+Eamd8eJKuAv8vIs60WuQTm/juQLZKzYkvNquSj7ds+6VxR+zf5o/l9T/OiqgTbZILKTu76COaOXhnw50iaI8KNUi+NUlAVADOJ9SGLI4d/3W3YhV3hReJVv1hw7XCwvGW93C9SVe3yznDu+f5DOpWEXU2hOu5W7TSC3cWvpFfSn+TJybypDufIjIoljc+t2WiIBT0/LvtoLszX6nqiGZ5/nlRw2oDnU+HNfXGOnVnHp3tSRgSTfnHu8vPWU9DgXNjjkuPwcM4aC6kj11QPz1obMcShTahdHZUeYsZD3p8Zk7cHJ+PRmHCy4GjPkzXxT2elVU/JGRmGyPl6X639I9qf7IaEe0QazLr3XvK3Yg2tMtgMaRAR1oFfvkhzwvmc= X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: awhEymJ2au9DKuagg9BjDV9T0YonZ/6HIrQLtDtLFQHtXBkMQvkwsHEPtr2NXsCIYnHHnUVeoMmKTy5vQTs7hfEysPWyDwXj/BXD+DHNAGJg2uBDPpb6SYMC9a5/9g/n92VpNGrW4vBt23FZW6hLxw== X-OriginatorOrg: hotmail.com X-MS-Exchange-CrossTenant-Network-Message-Id: c8a30892-2e66-47e4-cb27-08d988c8eb6e X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Oct 2021 12:57:57.7397 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-AuthSource: AM7EUR06FT028.eop-eur06.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: Internet X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM7EUR06HT053 Subject: Re: [musl] newlocale: Segmentation fault when locale input is NULL Thank you very much to both for the detailed explanation. I really appreciate you took the time to explain why musl behaviour should remain the way it is. I will proceed and fix the bug in GNOME. > In particular, here it seems to have found a bug -- what could the > application have possibly meant by passing a null pointer there? Did > it actually intend the behavior of ""? Or of "C"? Or if the intent > was > to have this mean "don't use a context-local locale", why pass the > pointer to newlocale and process the error (which could include a > number of other errors you'd certainly want to treat differently) > rather than checking for null and taking a different code path before > calling newlocale? Very valid point. Definitely something worth to ask the maintainers. Best and thank you again, Pablo. On Wed, 2021-10-06 at 08:29 -0400, Rich Felker wrote: > On Wed, Oct 06, 2021 at 11:31:29AM +0200, Pablo Correa Gomez wrote: > > Dear musl maintainers, > > > > While doing some work in GNOME control center for postmarketos, we > > bumped into a segmentation fault which is also present in GNOME in > > Alpine[1]. > > > > After doing some degugging, I figured out that the reason is that, > > through GNOME desktop[2], there is a call to newlocale, where they > > end > > up calling it with a NULL argument. > > > > newlocale(LC_CTYPE, NULL, (locale_t)0); > > > > In this case, "name" is passed to __get_locale in > > src/locale/newlocale.c:27 and then dereferenced in > > src/locale/locale_map.c:43, causing a segmentation fault. > > > > In the case of glibc, this is not an issue, as per the > > documentation[3] > > they consider it an error: > > > > EINVAL locale is NULL. > > > > Unfortunately, this is a difference in the implementation between > > glibc > > and musl, maybe due to the fact that the standard[4] in not clear > > in > > this point: > > > > > > The newlocale() function may fail if: > > > > [EINVAL] > > The locale argument is not a valid string pointer. > > This is specifically documented as a "may fail", not a "shall fail", > i.e. it's not guaranteed to happen. It comes from POSIX, and is an > instance of a weird pattern the committee tried to fix (but missed > some places it seems) where they wrote "may fail"s for conditions > that > already have undefined behavior (here, use of an invalid pointer) in > which case EINVAL would already be allowed as a side effect of the UB > without any further specification. (The same thing created a lot of > confusion in the past about use of pthread_t values past the end of > their lifetime.) > > > My personal believe is that adding a NULL pointer check in musl is > > very > > simple and might help not only GNOME desktop, but maybe also other > > projects in the future. This is the reason why I brought the issue > > here > > first instead of directly patching GNOME desktop. If you believe > > that > > musl behaviour should remain the way it is, please let me know and > > I > > will send MRs for upstream and Alpine's GNOME desktop. I am not > > subscribed to the mailing list, so I would appreciate if I am CC'ed > > in > > any response. > > The musl behavior should remain the way it is. My text on the > rationale actually made it into the glibc wiki some years back: > > https://sourceware.org/glibc/wiki/Style_and_Conventions#Invalid_pointers > > "The GNU C library considers it a QoI feature not to mask user > bugs by detecting invalid pointers and returning EINVAL (unless > the API is standardized and says it does that). If passing a bad > pointer has undefined behavior, it is far more useful in the long > run if it crashes quickly rather than diagnosing an error that is > probably ignored by the flaky caller. > > If you're going to check for NULL pointer arguments where you > have > not entered into a contract to accept and interpret them, do so > with an assert, not a conditional error return. This way the bugs > in the caller will be immediately detected and can be fixed, and > it makes it easy to disable the overhead in production builds. > The > assert can be valuable as code documentation. However, a segfault > from dereferencing the NULL pointer is just as effective for > debugging. If you return an error code to a caller which has > already proven itself buggy, the most likely result is that the > caller will ignore the error, and bad things will happen much > later down the line when the original cause of the error has > become difficult or impossible to track down. Why is it > reasonable > to assume the caller will ignore the error you return? Because > the > caller already ignored the error return of malloc or fopen or > some > other library-specific allocation function which returned NULL to > indicate an error." > > In particular, here it seems to have found a bug -- what could the > application have possibly meant by passing a null pointer there? Did > it actually intend the behavior of ""? Or of "C"? Or if the intent > was > to have this mean "don't use a context-local locale", why pass the > pointer to newlocale and process the error (which could include a > number of other errors you'd certainly want to treat differently) > rather than checking for null and taking a different code path before > calling newlocale? > > Rich