From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-4.5 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,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: from second.openwall.net (second.openwall.net [193.110.157.125]) by inbox.vuxu.org (Postfix) with SMTP id A637628E53 for ; Fri, 26 Jan 2024 21:17:15 +0100 (CET) Received: (qmail 17875 invoked by uid 550); 26 Jan 2024 20:14:59 -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 17843 invoked from network); 26 Jan 2024 20:14:59 -0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=U4F4an3JF4J/jZhDINdostNfd53vhAKrxrnOK+y3a5FcvKkQui0rpwQNZjJprN2hgRRoThYoRTYRBZTxToqBknjIQ3S1yOr/37180DpTnYJWuwIaJLooeFJeP7vnhb+pPWV1Fm2hZtW/l2KLbU7NMmO3bi1eCtKtXMRhOfrZz+ACPWKuOM6Q4xzln8RSxyYwQofz2Zf48clm9qZQeXOAol7Y7aAAlfj4YzP/qqylsr6Nm5hgxW8ZJfh4fA0dBTGt81mH6u7iN7uAv6c0xI3HSvy3gV8hOyQfDwnEAI3B299b1U9gT2c4aOZMRGkO1mnNJ+CUACYHRYMpxhZ2/BJC/w== 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:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=uQKiTCs3nxafuXkEjzXw7617OZK2a74NW8HMycDJUvQ=; b=ZL0YUBvxAd1XToupgi4WM4IVL6p8nwIA4GucixEkH9WpnrOYdMK3YP2XzlmdEeZMr5w2CeeF+GBeFt28RyMU14e9GtlCsKStQlusdwqfz24tNtBNgLS68KSSFuR3Ub4lT++MZaz1wxGH7ZfYh635ma32u8WapJWHwfZCBckB7urT+nZ04mOOZdyuubynXoQmjqLx2aBFnE7HBcYy9u7PLiC15GiKTB6F2TLDqEV58vgrssKZcFPMSpRmUAs066TV7iPoRw6Cl6w5ucUkmzIxergIcG+L1NL838gmpjeGJTySgH5wv6YeJPWTUd7V4RZGHc6VVai7g3EKgOnrjtaL6w== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=microsoft.com; dmarc=pass action=none header.from=microsoft.com; dkim=pass header.d=microsoft.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=uQKiTCs3nxafuXkEjzXw7617OZK2a74NW8HMycDJUvQ=; b=eUx40pgDcU5FDSAoFxqkbYFHKEuIDzFweRUPx04iSok+uY9y1XwhLAVEW1RPw2piEQEn6i4hxl89n7oykl6dLSnWSQ/V/tH1yqZKdE2XusSklwWXJrPH9iDzeGeY0xOy/GSxYKREwaoqzPFEkqAWlPT7j4TVaFLsKUtqQDe2lV8= From: Andy Caldwell To: "musl@lists.openwall.com" Thread-Topic: [musl] RE: [EXTERNAL] Re: [musl] [PATCH] fix avoidable segfault in catclose Thread-Index: AQHaT5h2Dpt6TRo5d0eFNmTmNsHhDbDq7KxAgAAeRICAAURdMIAAC1IAgAAc/qCAAAzZgIAAAckw Date: Fri, 26 Jan 2024 20:16:58 +0000 Message-ID: References: <20240125070950.28673-1-ismael@iodev.co.uk> <20240125212548.GL4163@brightrain.aerifal.cx> <20240126172716.GN4163@brightrain.aerifal.cx> <20240126195701.GO4163@brightrain.aerifal.cx> In-Reply-To: <20240126195701.GO4163@brightrain.aerifal.cx> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: msip_labels: MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ActionId=25ab1019-9c39-48a7-8d30-ff8f98a995eb;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ContentBits=0;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Enabled=true;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Method=Standard;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Name=Internal;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SetDate=2024-01-26T20:03:24Z;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SiteId=72f988bf-86f1-41af-91ab-2d7cd011db47; authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=microsoft.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: AS4PR83MB0546:EE_|DBAPR83MB0438:EE_ x-ms-office365-filtering-correlation-id: 0d1219f5-8678-4e2c-041a-08dc1eabbfb1 x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: FnssRsAGMSStdbuJsnvp8VlS9rEnMzE5jbNxd9jOd/JIXyUbs0O5Z86uvSZeKVtTGmj2XyxyC+DkNPj89ZBVwRwssuitr6j5I4kGHbZiRr7hG5bKAIklTPjT6nzU83NrRNXt5TOv6W0hL6DJGLlNVo+0YKNJLNb6qvMs11tlIRRJSbDXBZchazcPJE0IcWBeYjvGrsfxFPK6mc3uVrT6dt7AgoEWOZ+H88zSmWeiOZOu14QvNAjZsC7RVE+bQ+J7OJSJ6S84FqCh4kB/vgYW5KBs4lMrrUQzYoF2xhjFj9502RI2f7VpDIDheTzt0SvFSrrvyIzidT/cGuf3vyLoC7g43CO0BfZRJJy7ZY3BrRncRwrORbs4cbJaUiNKCBQss3BV9WLYGuZImg5LRkmxAK5jurEsjhgFEHNOcOrEt7Q5BFjrp0yEOXObL95pFJWGlMpoE8J1C5AIrdC0tE3rjKQo41zWQx1vm5aKo+qoiJyqeQnqkkOfwmkVzIc/5Yi96oJ4yOBHXEdQPHk/t2eHLp/UbFc8RJ7WxqY8L57DMEL50QOrv4olYrr0uc9TZ7F9r7RjfBp+3ZFQEIbeJnHdIDdO/NldR20xPVeXBiZwQGI= x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:AS4PR83MB0546.EURPRD83.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230031)(366004)(376002)(136003)(346002)(39860400002)(396003)(230922051799003)(1800799012)(451199024)(64100799003)(186009)(41300700001)(55016003)(122000001)(82960400001)(10290500003)(38070700009)(8990500004)(83380400001)(71200400001)(9686003)(6506007)(7696005)(478600001)(82950400001)(38100700002)(2906002)(52536014)(8936002)(966005)(86362001)(5660300002)(6916009)(66946007)(66556008)(316002)(66476007)(76116006)(64756008)(66446008)(8676002)(33656002);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?8rietmoPPFNrzxgQiIyWQFCoHoRSD3UCIvAggOm37gKXweygq6yz+7z9Z7dX?= =?us-ascii?Q?UU+AC/Zb/WmtMB64Cs4zIUV9HJ601mpD7pyevOg57+M80LmpHHtINVtnzg3K?= =?us-ascii?Q?20w+Voa/pDLjVinssO2uykFmmiFUURxMAzem9JEBJppdBn00rD8rFrmowBZr?= =?us-ascii?Q?dBDNbFoLnea+h2yOFujfU49BTy6lznb5MTnnZGWeclgjkjLddCNB4PkU6zM/?= =?us-ascii?Q?Ixi3V0kP0bCh4zYBNe8jJUHj2rOhOKFxtlaVrHgzCDFmGO3ScKft1sSB54hd?= =?us-ascii?Q?9cZRYtsCV0nWI8SmF2udQu4F5ximuUaNmnd076+eBklBpjQagxfGSv7Hq3Qm?= =?us-ascii?Q?PEYnZpiqUYhuFY5O6R97WSwPzVHYvM8tFOeCftQs4STn09xLPEsS0QDhMVPa?= =?us-ascii?Q?P5N1N2N8GLyYOf+Wb2RHc9QhmEXqGzgSzJvXU9x6t2QSHHop0xpEbCvB3KA0?= =?us-ascii?Q?KEJd8c4lszY+42BLAkRaO5qpjCNfabeaut6TTYWnIWDqP2rX+JCF27rhfS91?= =?us-ascii?Q?OvsBCHQb2tddSJzSczO/FASg5zzgGliTD5MAjxgzK2VqZnZHlBpQ+4+o90Ai?= =?us-ascii?Q?OJFDjFAwt6REk57780/VFJo/QcHRaImvs4m3r3kIdX4z85bq8XzD0hEn0u+m?= =?us-ascii?Q?xmC6Zd62bXF2Indp0668EAo2+EQfq4PDUkanIEE5yvJusXtLLY3frRaW+No6?= =?us-ascii?Q?ysZK3SyEnfVFne11EakCFW7zK2d6bS0+OZMz4KsxDOU+Ev+2vV23K8bwz286?= =?us-ascii?Q?27n9iApJhUyJ5viw5xpPt+B7jwp1ZCEyAkuByLj4wJV0DJ1SBpSciG9V0oQh?= =?us-ascii?Q?Pc1oOESfsukPDjlAhTQWM17q9aO8Tax2Py+m82zH3/+sCsiIIxZ2WLdm4s3x?= =?us-ascii?Q?StLXqZwmlWLMu+orUUnHNKufUATZ0evUSxm6Vfl9xIHma0JGXzKk1eoeUfEf?= =?us-ascii?Q?aRhTWekR7N/3CYeqsQg5HeMMYaahQpl7Fg6KlAlKUHgIajNdzJyC8Qm9j0E4?= =?us-ascii?Q?rsmjPU8aCyMdWKEONMa1PEQs+spXA/daUv520DUcnuhO0Qa59O8UifhvVHHv?= =?us-ascii?Q?CoJS9qC2R9a0GQM3FYxQX+fcTyeTy+ItFOEPVUDFne4CIclQ2a/HrycsTFIt?= =?us-ascii?Q?Rw0WMmQTKxTiUM+Am9HwAapJCNJokgy2JiTyawE3hp1xIpEk7+iUFv9/cSP+?= =?us-ascii?Q?nzki28a32zQ2sUwlMYCymVqelg6Qxfqc/0ADyQ5WJewBqsi8KzRtVEUJWshI?= =?us-ascii?Q?T+3pRR3Av6C6WJwoc1GKPsRnbcYkeLaU+7rJdsZyU3Y/bAvejMIAp3aNkixD?= =?us-ascii?Q?GTQcCd0LGXQKJ/EpBvRJ72mSp/4o2712ueKo11yDJvtMJ1GTu09fPtyJ4RpJ?= =?us-ascii?Q?xsf2qZ4y37HS+wUWlgo4N5zeyTEXyaEi7A5X+dji7KOl7XZqOxEbzJ9RlvhI?= =?us-ascii?Q?3zxqWGdpdfe20EfVtLf/Y0mxN+3Kyqhm8z/HiGf1PRYoRSNFG/ySfOC9t45X?= =?us-ascii?Q?aYwZzOkklc18sZQpW3rFISEB+iAWo3QjRUs9GvJoJytx5p/+zeyuSjBEck7U?= =?us-ascii?Q?pf62r8EsmrvSvBEqp8xlqrQUkpmtKUlyXsYcr1xE?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: AS4PR83MB0546.EURPRD83.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 0d1219f5-8678-4e2c-041a-08dc1eabbfb1 X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Jan 2024 20:16:58.3578 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: JaJn4wH+MwyRJ/nxHiLs5fBQO2Ug2qE3eaIvihfi5MgUseBySvD+uuT0Qq1p7IwPOJ82cbZCuythjXIB2ZV4v5nbp2JWPQhdR2XwjOan7QM= X-MS-Exchange-Transport-CrossTenantHeadersStamped: DBAPR83MB0438 Subject: RE: [musl] RE: [EXTERNAL] Re: [musl] [PATCH] fix avoidable segfault in catclose > > > > > > > And it has been musl policy to crash on invalid args since th= e > beginning. > > > > > > > > > > > > The current implementation doesn't (necessarily) crash/trap on > > > > > > an invalid argument, instead it invokes (C-language > > > > > > spec-defined) UB itself (it dereferences > > > > > > `(uint32_t*)((char*)cat) + 8)`, which, in the case of the `-1` > > > > > > handle is the address 0x7, which in turn, not being a valid > > > > > > address, is UB to dereference). If you're lucky (or are > > > > > > compiling without optimizations/inlining) the compiler will > > > > > > emit a MOV that will trigger an access violation and hence a > > > > > > SEGV, if > > > > > > > > > > In general, it's impossible to test for "is this pointer valid?" > > > > > > > > > > There are certain special cases we could test for, but unless > > > > > there is a particularly convincing reason that they could lead > > > > > to runaway wrong execution/vulnerabilities prior to naturally > > > > > trapping, we have not considered littering the code with these > > > > > kinds of checks to be a > > > worthwhile trade-off. > > > > > > > > > > > you're unlucky the compiler will make wild assumptions about > > > > > > the value of the variable passed as the arg (and for example > > > > > > in your first code snippet, simply delete the `if` statement, > > > > > > meaning `use_cat` gets called even when `catopen` fails > > > > > > potentially corrupting user data/state). > > > > > > > > > > I have no idea what you're talking about there. The compiler > > > > > cannot make that kind of transformation (lifting code that could > > > > > produce undefined behavior, side effects, etc. out of a condition= al). > > > > > > > > It's a hypothetical, but something like the following is valid for > > > > the compiler to > > > do: > > > > > > > > * inline the catclose (e.g. in LTO for a static link) > > > > * consider the `if` statement and ask "what if `cat` is `-1` > > > > * look forward to the pointer dereference (confirming that `cat` > > > > can't change in the interim) > > > > * realise that `0x7` is not a valid pointer on the target platform > > > > so UB is inevitable if `cat` is `-1` > > > > * optimize out the comparison since UB frees the compiler of any > > > > responsibilities > > > > > > You have the logic backwards. In the case where cat=3D=3D(cat_t)-1, > > > catclose is not called on the abstract machine, so no conclusions > > > can be drawn from anything catclose would do. > > > > The original code I was working from was: > > > > ``` > > nl_catd cat =3D catopen(...); > > if (cat !=3D (nl_catd)-1) { > > use_cat(cat); > > } > > catclose(cat); > > ``` > > > > (i.e. an incorrect use of the APIs, but not UB in a "C99 spec" > > sense). In that code the `catclose` call is provably inevitable, > > allowing the compiler to infer properties of `cat` from it. > > Ah, okay, at least now that makes sense. But indeed it is undefined: > > "Each of the following statements shall apply to all functions > unless explicitly stated otherwise in the detailed descriptions > that follow: > > 1. If an argument to a function has an invalid value (such as a > value outside the domain of the function, or a pointer outside > the address space of the program, or a null pointer), the > behavior is undefined. > > ..." > > https://pubs.ope/ > ngroup.org%2Fonlinepubs%2F9699919799%2Ffunctions%2FV2_chap02.html%23t > ag_15_01&data=3D05%7C02%7Candycaldwell%40microsoft.com%7C7cfdfca2ce32 > 4149a5cd08dc1ea8f669%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7 > C638418958276620853%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD > AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata > =3D0lEpsKknyTB3xULpxaOBZS6UIzI1rCG4D86U9z6P5Xc%3D&reserved=3D0 > > So I guess what you're saying is that, in the case where an erroneous pro= gram like > the above has undefined behavior, the compiler could make a transformatio= n such > that the effect of the UB is seen at a point different from where it logi= cally occurs. > (This is the norm for UB.) In particular, despite cat being -1 from a fai= led catopen, > you might see use_cat being called with a seemingly impossible argument. Yes, this - the details aren't particularly interesting but the key is that= "invoke UB" is not the same as "crash/trap". I'm also contrasting this to= the comments in the glibc wiki and Markus's synopsis (from the earlier ema= il) that "it has been musl policy to crash on invalid args since the beginn= ing" - in the face of UB, musl (and presumably also glibc) _doesn't_ crash/= trap, nor does it "fail early and catastrophically" it instead "propagates = the UB". In debug builds these are often equivalent, but the specific path= to UB might not be seen in a debug build, and only be seen in production w= here the non-locality of UB effects are at their worst. > Exacerbating the degree to which UB can become non-localized is one of th= e > expected effects of LTO, and arguably a good reason not to use LTO for de= bugging. > I don't see a lot of value in trying to prevent this in isolated cases wh= en it's going > to happen all over the place anyway for other reasons. I think that's reasonable, but it disagrees with the "policy" as described = in the glibc wiki/your SO post/etc. (maybe I'm being pedantic, but I read "= it shall fail early and catastrophically" as "it will definitely fail, and = in a useful way, even in optimized builds" which isn't always the case and = often can't be). Maybe I'm simply suggesting that that position needs some= clarification/caveating?