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 60702232F6 for ; Fri, 26 Jan 2024 18:13:29 +0100 (CET) Received: (qmail 26110 invoked by uid 550); 26 Jan 2024 17:11:13 -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 26078 invoked from network); 26 Jan 2024 17:11:12 -0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=cw9nh7FMnEr18QUeGbFte2FYyPNeDXNmIaCpWZ09GwOaF/3L/0Huhx6zdE0T9EQ/GzB4M1P0bJfGkHTIoDX/C0vVvrkGHwOcGTj1xXYrbiOs9lh5QgPRsVWh7eB2nTizKrouhRUuGswsTMfsVrNJWK+GteFL0BO4NjPl3jwrMeJ7ImainJQinMtQUZZBl8wv6qToFIiByJ9/Y6lzz5SZRZwjIqbSVbJSQ/N6YOuIcZ/S7aH3N55kgw9+bw8cePJuuyASWm/p5MfC8BlicBlFAxYtX7mWvkQWTV/ZBjLUBy7Nx9H05Zp9L6luW4Yj4AyYGIclXaO0gK4H4q2wafatiw== 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=gulkTPT1uzqEXbfQqN2Z/LhwYKt8Z33FO81SNIJFFBo=; b=VtKdCPsGiZ6kW5Qcx094MS6+PHNe1Ghozf70vs0OpDnEMY6OTpRHFfDeeFTeU7C9MY9PMunzgcoyk5HnERPgDbV/6i8/xWqjmiSJ1aIxMdT+r+7hMzC3oqTy9JWdCt7tlWhe35BGKWPYWhxkKL2dfPpLdutw4vmx/a8ven4P2VbuB0NyXsOio8ZB6pLwiXI1nB5m4PQLg/EZI42sycxILsRJ1h+2HE6Gk8tmCi6fDmljKZVUr9J0uo7MQTBYW0A4hRGNXdDqZnSbq1NeiSpvGs54IO4I8T4wmj3/5RyMiGsJO+d19HeVkxrShUY4WUaf5Xex/EOGsHn2ZrkCsjGyIg== 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=gulkTPT1uzqEXbfQqN2Z/LhwYKt8Z33FO81SNIJFFBo=; b=CxyYkEABDwg2jfmZyIugpbzcNF1VGYp4p9XHNJNfMcRixpPxka5rFjFdSGW22KQfdA3+340LCm9iezevxQQbt56ONSyFGmvqH0o5+lk8rdAQuh+NcvaESILdf8OsiayEgKVns5lXM7h0GKeyP7LF1d702UngnzPw0XbUguJimDM= From: Andy Caldwell To: Rich Felker CC: "musl@lists.openwall.com" Thread-Topic: [musl] RE: [EXTERNAL] Re: [musl] [PATCH] fix avoidable segfault in catclose Thread-Index: AQHaT5h2Dpt6TRo5d0eFNmTmNsHhDbDq7KxAgAAeRICAAURdMA== Date: Fri, 26 Jan 2024 17:13:13 +0000 Message-ID: References: <20240125070950.28673-1-ismael@iodev.co.uk> <20240125212548.GL4163@brightrain.aerifal.cx> In-Reply-To: <20240125212548.GL4163@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=80fa9ae7-fabb-4633-bac1-63ab4c06ebb6;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-26T16:46:45Z;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_|VI0PR83MB0622:EE_ x-ms-office365-filtering-correlation-id: 48e60616-4308-40fc-6790-08dc1e921416 x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 9QPsWeNheqvLiEpEx0kH5lECzt74/ZhzGTttZ/DjZHUkW0e+pJ5Qnr2cbvvUu5jFSPEAOm2cvF7ULPG0IvzU11e1ZF4Umi0w7d4EwyCQaq6e1gNHKtfgs/n6VvAb/JuMvVVF4zkSKNcbjgp7kc8OqrRfSORSY6LjRKH6deyGbG6vM2HIHhXS+TjKml13AkjzD9bpCEvlNCvZlOkRWYOx/8wEs4W5UMfnXIJCRGuvJRjUV79iioOQ/yavWqdCwG7B7uIBurzpsl7E+aI15CjDwLRyhf0FagHVsbh6Xwopsj4gKPzEWLn10b+QPHZKK8ljfPOB2gDUtRrtf3LbpCMIi83Udoh71lLNqXF6v/UFluwVDF8jEWXj/7dEzmaCmQroUFFvANntwIpBfCUX4l71mPb27mpkIN0y/1kTPhrhK8k09wuK+C9GvbutzekasFk9HsIhCA9ltgumpenjK4VTUGo3Gxqr1cpG5c1iLgtWqnSZsmYAAXZwyvFk7uJa7BixR0+TVBGve2eNj83mx478OoaIOGuOAePj8xjRFBVDQeqjdWHLFYdsouxa+wZuF9PdVXoqVOYmpueP29+nhcnbGqF9uSzpNoSQpXaGF8bzLvw= 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)(39860400002)(136003)(396003)(376002)(346002)(366004)(230922051799003)(451199024)(1800799012)(186009)(64100799003)(41300700001)(66556008)(10290500003)(66446008)(9686003)(64756008)(83380400001)(55016003)(6506007)(7696005)(38100700002)(71200400001)(6916009)(66476007)(4326008)(8676002)(966005)(86362001)(52536014)(122000001)(478600001)(82950400001)(82960400001)(316002)(8936002)(66946007)(76116006)(38070700009)(33656002)(8990500004)(2906002)(5660300002);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?vVNJBaDOJeg0Fk+XAimLZswSK8EoqkrSj2xaqFbJ0KNtKLqxkMklty5c1/RR?= =?us-ascii?Q?DrrJPkK/zAhblYQz0se7DzzMAiorVnjhdR3WKCwL4CuwppQtBmlUE4Usb3cg?= =?us-ascii?Q?oiXPDgJ51kFVC5yWhVre/h2ylAYu7eyDG5rRScXBt2wGZzEHccOAskZ0Bcfy?= =?us-ascii?Q?5kxis0EsAxaBuO9no67Sx2cy9CDDTgVABX19WZzWOvgiQlywF08bQQ1957Od?= =?us-ascii?Q?X9xYrUdUwq6WzLDVIAt/cyuzTImPHV35rLOyvWKlUSy9LxPxaAu8iKWTYqfy?= =?us-ascii?Q?VlToJBE2PYUGGvHG3OE7tCAZQXSSvlI310JZ0hANZMV7qHF1Qjz/0St3Ip7q?= =?us-ascii?Q?bWWj7f7mBAsuCf4+hns6pZ6hiqZzeC7VdYLBSudtEb1iKlIfFRtz5yt9EUX4?= =?us-ascii?Q?Ff8EWwWTG+/CjXSJv4yvXkAnrFWiT18doyhsU7WuN69LznavOiurVYFKllOM?= =?us-ascii?Q?+rB0I1MpKZCcrZKeH29jbhNff/SOCtxaoawX2exS0rC5zddw6hdm6zqrHwYD?= =?us-ascii?Q?GggVKBsaXFQUfPkKTcp/8LhGGR9xjGxA8AqmrpkK65R54p3SBRca//7QRkx8?= =?us-ascii?Q?WkpKkHaG7Tdjd/QtDF8v9XpANe9DcNbq9MJ1QhzV8XIVpaPt/zNns9+6LIlA?= =?us-ascii?Q?t9CpAKu1HteyAJhfX7j4btCgG3FDFgHca91gADq/61vtE5kBIisWbulvL2BC?= =?us-ascii?Q?vGSV0+/hTRfMe1g2QkYUKy8jiI0S7sgpO3jmvS4H5VSFl2UYwB/1R3U2OeFy?= =?us-ascii?Q?DshxemNkecuyKaXtRO7t0fRJU8ejgLd7m59+OH24Y+P/mrznkOYafyF6HKf5?= =?us-ascii?Q?kKWnwPalXfKtblnNc82RIcIJZ7Vdcy+BKH6kqAVl2/wYd33IP5UffrSp6C/+?= =?us-ascii?Q?6FcikHbInXJvq1+oUPK7QFYZBIBC3dpifzCra1Iz7n0/xpu0RQLp5EyBR//5?= =?us-ascii?Q?X/ppf4PKEHBn9YLlmTtHTMdrJKDwO+imgrTCz7w1U8WbFh7fvjgkFCdH7ZZn?= =?us-ascii?Q?w3fpMuYQMxCLZbUJlnBoOLXHs5PBER7BE8ig137jn4iSui9iAS0bseL4PnM1?= =?us-ascii?Q?b8a/fn9W9xbaM/uCPTvWpMPYrwoEJ1Vn4TDKZce6EFpHU3HDVIdeophpsUB5?= =?us-ascii?Q?YMPHbjGHjnWXWPxvURAEvaE00xFE9phCD7r/QEw+V8UsErfkl1VlYn/Px/lA?= =?us-ascii?Q?vXqGfQCouhfLv9KqduiL9IV42nmSIFepbieG/eAXu2qGZ1GMDvjKD+VucSVG?= =?us-ascii?Q?1g4MuIP09G5ocGFTTa+CIbzeHyrN2swLInYWUlCcD81EtgGrLfYDPJnMyb0m?= =?us-ascii?Q?S+WoCie1qhTtanu//r3mBBshpA8YyvCuQq5XuiVpj2KqQwBvJdH4DY/wnndN?= =?us-ascii?Q?kqo2FZzaCYW74H6Ze+Rvb5v0WN7AOKqoOKHscRa1cz1QRkWLmFOUoH7n2OQc?= =?us-ascii?Q?eH7EB/8uj4UP4IVGc68+oN+XZbSTsnUPFXv+cucxNGTZETyNkO3ts9+7CYCl?= =?us-ascii?Q?Ycm54HOi4ruoi4FW1kKf9oeXQkZpttdjRvhmyZylmnuQnwAXlOXQ4SFp/QBq?= =?us-ascii?Q?FL+3IEgQ8AFFak4PP/FqWZUlbbqg9cYmEP09yAzN?= 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: 48e60616-4308-40fc-6790-08dc1e921416 X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Jan 2024 17:13:13.0627 (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: mmik1V7+wQAFZE1Vb3C2Kn2lR8ArdAXqBqoSVsbHXalL8tadqwBqz6gTLUxSFsbMzsqC+QXVAcfzGrQwQhi6cUnEKQlJoKCOHoRwm41QcZg= X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI0PR83MB0622 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 the beginn= ing. > > > > 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 >=20 > In general, it's impossible to test for "is this pointer valid?" >=20 > 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 consid= ered > littering the code with these kinds of checks to be a worthwhile trade-of= f. > > > 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). >=20 > I have no idea what you're talking about there. The compiler cannot make = that > kind of transformation (lifting code that could produce undefined behavio= r, side > effects, etc. out of a conditional). It's a hypothetical, but something like the following is valid for the comp= iler 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 chan= ge 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 responsibi= lities As an example of exactly this kind of UB-at-a-distance happening see https:= //lwn.net/Articles/342330/. As compilers/optimizers get better the scope o= f fallout for UB is growing over time. > > Crashing loudly (which requires _not_ > > invoking UB) on known bad inputs (a test against `-1` isn't exactly > > expensive) feels like it meets the "musl policy" better than the > > current code. >=20 > Letting the caller-directed UB "propagate through" to corresponding UB in= side the > implementation gives maximum debugging visibility of the root cause of th= e crash, > and lets whoever's building link up their preferred form of instrumentati= on (e.g. - > fsanitize=3Dundefined). >=20 > Did you read the linked text > https://nam06.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fsourc= ewa > re.org%2Fglibc%2Fwiki%2FStyle_and_Conventions%23Bugs_in_the_user_progra > m&data=3D05%7C02%7Candycaldwell%40microsoft.com%7Cb8a5e6447ca64d9484 > 2d08dc1dec2e01%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63841 > 8147445459351%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQI > joiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=3DqxIHR > sJ3nCRiYxTSdECipLeIxi9khnd7R5HOyr8RCvI%3D&reserved=3D0 > ? >=20 > Yes that is the glibc wiki, but I'm the original author of the text that = was based on, > which was in turn based on existing practice in musl. As written it's abo= ut NULL, > but the same applies to (cat_t)-1, MAP_FAILED, and invalid pointers in ge= neral. I did read that post (in fact that's what prompted my comment) - and I agre= e with its sentiment. Unfortunately for that post's goals UB is non-local = in the face of compiler optimizations making trapping reliably when functio= ns are misused near impossible (though I acknowledge the point about `ubsan= ` which I'd not thought of). If `libc` functions invoke UB then all bets a= re off, and it's near impossible for them to validate their arguments (e.g.= "a non-freed pointer" is not detectable in any feasible way) in order to e= xplicitly `abort` or similar. Maybe the intended policy is to order the co= de of each function to "propagate" UB though as early as possible and hope = that causes a fault that can be debugged (or that the user is using `ubsan`= ). A