From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 128704 invoked by alias); 25 Oct 2016 14:04:28 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 128690 invoked by uid 89); 25 Oct 2016 14:04:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.2 spammy=__rawmemchr X-HELO: eu-smtp-delivery-143.mimecast.com From: Wilco Dijkstra To: "adhemerval.zanella@linaro.org" , "Andreas Schwab" CC: "libc-alpha@sourceware.org" , nd Subject: Re: [PATCH] Fix undefined behaviour inconsistent for strtok Date: Tue, 25 Oct 2016 14:04:00 -0000 Message-ID: x-ms-office365-filtering-correlation-id: 6da71ebd-336c-4f88-7b6d-08d3fcdfcc20 x-microsoft-exchange-diagnostics: 1;AM5PR0802MB2612;7:EfYZG4q6KWqHfVo0s+W7d3B0CDKX+ZqXHj8ZajEWGDYe7VYWXoLvGthbLC7VR4x2hlC3bp/7UbhbcevsQhdAC1jdz4mehH8zL8obpz2qYJrLkx4hac9z7UoDLhQ4+Mp3Yk1S+kvW/r4AxoOG7yXcTx0UaNgUaxjrwGpKkC/e5ctCY3LcmP8N0n/Q77PoD44l8vFXVrsTmWQn5qmVKRgXgZFK+SxJHnWNlTRc5Wi2HX8aWUkmS1NFIFEXB7SAGCHVSUQOHzXTZWJCZ7NgmHfwmoqsf36oUkWBHT/X3gnYMLavnPg+0DE2OnmndPd0fNh/n7ZblaWk77uluesxM5t6P7be+H2/tIOV72GE6FhvMWA= x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AM5PR0802MB2612; nodisclaimer: True x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(6040176)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6055026);SRVR:AM5PR0802MB2612;BCL:0;PCL:0;RULEID:;SRVR:AM5PR0802MB2612; x-forefront-prvs: 01068D0A20 x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(6009001)(7916002)(189002)(199003)(10400500002)(122556002)(54356999)(97736004)(50986999)(105586002)(33656002)(68736007)(5001770100001)(189998001)(81166006)(2900100001)(81156014)(86362001)(8676002)(106116001)(5002640100001)(6116002)(5660300001)(106356001)(3846002)(586003)(305945005)(2501003)(7736002)(101416001)(7696004)(7846002)(4326007)(9686002)(76576001)(77096005)(2906002)(102836003)(92566002)(3660700001)(3280700002)(8936002)(74316002)(66066001)(87936001);DIR:OUT;SFP:1101;SCL:1;SRVR:AM5PR0802MB2612;H:AM5PR0802MB2610.eurprd08.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Oct 2016 14:04:12.0077 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0802MB2612 X-MC-Unique: msPcp9VnNWOeB8cqDBfH6Q-1 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable X-SW-Source: 2016-10/txt/msg00422.txt.bz2 Hi, + if ((s =3D=3D NULL) && ((s =3D olds) =3D=3D NULL)) + return NULL; What is the benefit of this given: if (s =3D=3D NULL) /* This token finishes the string. */ olds =3D __rawmemchr (token, '\0'); So in the current implementation 'olds' can only ever be NULL at the very first call to strtok. To avoid doing unnecessary work at the end of a string and avoid use after free or other memory errors, this would be much better: if (s =3D=3D NULL) /* This token finishes the string. */ olds =3D NULL; Setting it to a NULL pointer (and not checking for it on entry) causes a cr= ash so any bug is found immediately rather than potentially staying latent when= =20 returning NULL. The goal should be to make bugs obvious, not trying to hide= them. Btw strtok_r has more potential issues, the reference to the previous string may be a NULL pointer, and the pointer it contains may not be initialized at all, so it's not useful to test either. Wilco