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=-3.1 required=5.0 tests=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 5C78021365 for ; Fri, 26 Jan 2024 20:13:15 +0100 (CET) Received: (qmail 11516 invoked by uid 550); 26 Jan 2024 19:10: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 11481 invoked from network); 26 Jan 2024 19:10:59 -0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Vr+Gmz7pH8j3gBxHayYvB9JRLt3S3c/MQ2TkJB/69cZySe0QtvihUuUwCnQ54Nuvr3umd8VP+xPaFwcuJba0WBtZPsWNBwN2o7yfC4whVQNZLxEcmJm6YVKafd86Wrjdh/Xja4d2dlGoP7p68+iCr+VwymvyVyNHLM36+ED3iD92w37ze8anDGllrbyR3OJeHE1DYCT7MdrsD/s4imdKKtl8H/42mVzP/YZNavpJuBch/9ROv4iEEhxvdxZ+zNGN0aOFv6lnRH3OZiA/jXX4r/yTx8V8pazG1XH0K593A+wf4wG/ZlooKJfyv1drlm7gSFrGk/tod9Teenj0ENo0Rw== 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=i48ZMC4r67RL6Pr6Qi3Gx0Q8XBvgHCs0E0b/FlbXLlI=; b=Y07kM5vvimnIYZmS/vZSnCWA0JkUCchKY7q/kPzOlhI9VkXvH7oCpDCKISpFtg6BALqgZF6KNhRAbZ3lJA+lTHbbX8Q2vl5sidHJg2s1itZrmoHXCDN3Uzv91VdrdXjumYV2fWhBu7HLdkcbm6NChPGTov+sDgbjoUyA2zN0Sx/Sz/g80mr+1AxuxAVwfvDV6w98seCO404rcRQijMOXcmUPcyRYhtwudSmnfThxKNM57mn+Y6cA4izDMBYZSBKc/MjTQt3MoDyuqbxzVVuYnb2fpZ7CwfIGgWn+Kri1IVNgDYkmyquG8oRhnAdkYG+26NtW7NszEK3EF8p20Mp60g== 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 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: AQHaT5h2Dpt6TRo5d0eFNmTmNsHhDbDq7KxAgAAeRICAAURdMIAAC1IAgAAc/qA= Date: Fri, 26 Jan 2024 19:12:59 +0000 Message-ID: References: <20240125070950.28673-1-ismael@iodev.co.uk> <20240125212548.GL4163@brightrain.aerifal.cx> <20240126172716.GN4163@brightrain.aerifal.cx> In-Reply-To: <20240126172716.GN4163@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=1ffcb6d2-8565-4b61-99d2-05d0e63a9bd8;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-26T19:11:02Z;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_|VI1PR83MB0430:EE_ x-ms-office365-filtering-correlation-id: fb271720-4ee5-41a8-2866-08dc1ea2cf51 x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: vdwRzIPZBBOgcxJTo5QpSF0HZx1nMx0Uo9rO6XohSpwmoF9eZHfQ6Y15wObpn2YFNFXtB1vcUXPnAoyzxzT1/+iNHMb9XnGS2lIIidzTTKB+LLPtcAb0xwhFcyUhaKMlYH0DC00jZykpdDEfrpWrXb6t2hn0jjJhMJrqJ0oQS4wItr1tGuK1b1BcnfZQocU31BxceIRpSoOwVOAlAt3Dp0e+KoN6PAsw8086rp0IuUZ4kxbmPsRgcxjfStUNRN3wrvFUz3CGpWRZyTewqUFE9Bi0KyfQOs9eJWc2a5cmISSwr1AE1fYJaMQajwycc4djZkxHYHdwEtiA4oc1mdCX/TDsA0bqrB3iLh/y/pq/TFViYg2RijDCc0o2679rKr0bmOJ3FmIf2is2PjwkQeBQUmPxjdx5IOopSkLEcrJnYEyuV9nsW0wnjpaElW/Rt/NCjCr2b2T2lZeqPDjLU3Fn2erpOqtUPH7/GQtsG0cwj3S+y20BbnA9+K29QTAO6Mh6X/g9JFW6MRJ4l+eMZPqytTzHzN+7dSnVAlf674TPvx4I8gZMjHYdfUVC8T+TQHZfqtBIkzey6E/3b7bZ6rs4tzOMUm8Va/uhPl2PeC7euRjuZkb6uulY2Nw46WLNLicz 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)(396003)(346002)(39860400002)(136003)(376002)(366004)(230922051799003)(1800799012)(451199024)(186009)(64100799003)(86362001)(38070700009)(66476007)(5660300002)(2906002)(8936002)(8676002)(33656002)(4326008)(6916009)(66946007)(66446008)(316002)(76116006)(52536014)(64756008)(9686003)(8990500004)(66556008)(10290500003)(478600001)(7696005)(6506007)(71200400001)(83380400001)(41300700001)(122000001)(38100700002)(55016003)(82950400001)(82960400001);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?vUtNcVMR+TN5CgbjI4E2JkYvGQYVaDBQ3xbniMPUzAJu65iRXj4TVM7z0wje?= =?us-ascii?Q?Q52GmEDykSrLCFNP5j6a92UNG7STZsKfz2dzGyi5cldld0ArxNzPlwJxcOrS?= =?us-ascii?Q?oyCIKCHIlsBVMtnkOdkhLAETKVi5MtAPa+7ELQsfIxSMcDDrWOh5cXXCaMJe?= =?us-ascii?Q?nter7vjYDv/6vrEYO6YeUd0IlhsGEQj816mKVgO/9xhDaTFjjLy8lrkEvsZZ?= =?us-ascii?Q?BEzoNlLxSpS7NIgYQlbv+nFgA1fCikXIbkPr9EgqmMc0C2BIN7jSTsJkgMpM?= =?us-ascii?Q?jTKqTexDmnh1wKMAgOfzw9EBP0OcZ686seilHIV0Ld/nD+5aL8ox0Wt+idf3?= =?us-ascii?Q?vguZ6mxkark9UNfybe3hn6qYxEHceZMJuN3qCfX+ILAdTgZWW+RF2QIAz+1/?= =?us-ascii?Q?EzgKdb5vSE7m0Ypc/ECEJ/aKskYmDGCh8zcObDLyW9ohG5+Zn4HSxvZHS0GF?= =?us-ascii?Q?FGQC0Ei3HOI/fpV58LEEpyaiOlPFBWC6beBsYa+g9Akoat/hmus80zj62M9f?= =?us-ascii?Q?W0QWqGkRvlSoLbP6gp4WPsIJUItKE9zNiihCJe9moBgLDBraZK9g9W+D2m8o?= =?us-ascii?Q?o3ccjjab4oUMBWoXaAsq2Ae3EoWe2GOfltmcnqLHP5fY5lg+M9Dmo6KySZk6?= =?us-ascii?Q?a7fWFuJEYz4CNetbYRz2oKFF2tsg2LGV776Py1+f72yiTOh61HlSnPJxfH/E?= =?us-ascii?Q?6c73p+O45g1KLKCZQM762ggFhfIW5tUacdCL2OLi/N2KK1bSbh+xy/m/To4Z?= =?us-ascii?Q?qmPvt082EbQwl84pMlY1tJFtyDAE2ocUO4Yidpjpc+5nOl41smbWSjWosFoT?= =?us-ascii?Q?OiGo9gSUqC9NmTMgI2TatwyEZ/GprDPic7tJqGiw0FIyVAP6IdsSN2SD8n19?= =?us-ascii?Q?zshy0r5yD+FmultyULhXrYmahsAEFYHxj6f0nEuSsYczwm48B8o0Aw+muYM4?= =?us-ascii?Q?EVyJHM6MJmPliEEzQ0dt3ECn7PqBWtWL+JnKiL94UkrzItTgN/RDgGSc+uMT?= =?us-ascii?Q?TSbryGscNb5pi2WyPRRynBGMXVR29XUchql0DkGUKTdKLLq4+NWjdS6590B1?= =?us-ascii?Q?JvqTaQXdllgxpWqrPw+SPzBGaYTayWRIJgk5A2xJaCldf2CDnFgaN68f5Hx4?= =?us-ascii?Q?h+2W+g+Wv0pIGPPGdxh3Hp68odAw4c/KcRas4Z/tBFZXFKnHtGKPXRsQm7nl?= =?us-ascii?Q?LBb1r650Vbkmn10mUTBWoATNpiM1QzCjQdMJJg8j1nHWvWP4zbUrzH5Omr+2?= =?us-ascii?Q?IkG6YJOYN0bsRwaXwFjR9pWFXgvZARqi8eflVm+hBv4tjVOyEE9y00h5jDAH?= =?us-ascii?Q?VXayAaxwrdTTDtKKRUoho0gx+wGZDeSu4GSiu4TcytwpSF5n71AOSunuANgI?= =?us-ascii?Q?ZkTNpXhq+awsOoftpGIFDyIe3PWjmbkVRldSDrylTkv6d0mYULE5a6YRmWQs?= =?us-ascii?Q?WuoFtqJ8y8BZPy/ZHmgBjpyNo33nXv8p/wfjMmPZbRx7WeTowwrpBcepPwSe?= =?us-ascii?Q?ZUfZsupc8d+Cum7VR4PD4Y0c1mmOaPAWuVD1qPOjqfAKXYZzsRjo8HJcY/S8?= =?us-ascii?Q?3b9cORXynHzA92jRAP+ZxQ88bnbsn7GlQ1UAV0dX?= 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: fb271720-4ee5-41a8-2866-08dc1ea2cf51 X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Jan 2024 19:12:59.1008 (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: oehvzSG4yGCqsL1CIxkh/2m2FD8To2lkUEVcAwuWLoIqIAD+HbTevjZVmyzU861bWoZQHuSTCILjurIHT5V4mFBZG/NfpVbDUyO6p+bdJF8= X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR83MB0430 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 be= ginning. > > > > > > > > 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 conditional). > > > > 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 >=20 > You have the logic backwards. In the case where cat=3D=3D(cat_t)-1, catcl= ose is not > called on the abstract machine, so no conclusions can be drawn from anyth= ing > 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.