From owner-svn-src-all@freebsd.org Wed Jan 24 18:19:03 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 6319CEB9FC1 for ; Wed, 24 Jan 2018 18:19:03 +0000 (UTC) (envelope-from pfg@FreeBSD.org) Received: from sonic304-35.consmr.mail.ne1.yahoo.com (sonic304-35.consmr.mail.ne1.yahoo.com [66.163.191.161]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id F1C1E6DB9E for ; Wed, 24 Jan 2018 18:19:02 +0000 (UTC) (envelope-from pfg@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1516817942; bh=x4DyohQ61xkt8TDv0BALYrA/BsgyN84jnEU9DNr/rxU=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From:Subject; b=rRywoMVe7EXMK5Ocm3Bo0egwCBFZkINjDkGbP8/ocFWzTw6MCoFYGO4mxz9jfBliOmkT/yAYxjt3kJtlvMJmYEEz7snnkK+J7qWxdrfwcZqvxxgW5SneKvyu781wOwXrf74OAV7V8VmSvjmiUPpxDHrt/LvmpWrNPQ9lWDcfCKAZNMZdWPQG3clTdZCrFNRuzuvJPCghfJACyEaS2JUuEecMc/AlU4QCGwy/yVZjO6DJEH/XGkzmq5OyxKKfC5/7ES6Dka0eZkd67WI2+cUuy9yFZK3eVeIU5eycze9ppXzSwvbrscZ4utJskvX8/BSaUMMYUzPutHroD9B5c84Gpw== X-YMail-OSG: BNPgUdIVM1mtoasaqmVcMI3_3SVbR0_fa8O6FLQcWfdbqsyQIQUBjq.FAovG34g dsH0uKjNdVDU8VmlaaswawSF003XJpomQaGn6_d4fh3KVAcVF4H__IRIU.gaRbdfr29.1A9e.5ru JObQBQUQ.Ej9DYAdt.iSc1w2cGERLffECGKGF8ydHUQFdTIR_3Sda3Wqm3KW6617Ec5Eabx3ajlG R_RmVeFStBA_OB68.awGJnn3tn_yHWG58hTgypxjFZHmSC.El9KSPuCnlL0jjahWoJPIAm_qw288 Mz7nsclqRmiNtPjcCJK.._lZYOuSxAfVEt2m5KD_A_c1u0YIDYYxbKAlADWO0cuP0jy.SMOkE7tj d2RW5fRRiwjGXi7UtNwATLz_jr5zpvVoioCkppUPJE2Mp0aicWcVBdRHbFsXlYsNHhh.LPnQoAXl IU8ey6hWRmTpv0lEWUAAJ3u_s6alqTz5q5et_CD_ohcjFYeEm5mTuu4P6zsTMSwjPwcxV3Rc7ZGQ Jfvgz8yh7mg-- Received: from sonic.gate.mail.ne1.yahoo.com by sonic304.consmr.mail.ne1.yahoo.com with HTTP; Wed, 24 Jan 2018 18:19:02 +0000 Received: from smtp230.mail.ne1.yahoo.com (EHLO [192.168.0.8]) ([10.218.253.211]) by smtp403.mail.ne1.yahoo.com (JAMES SMTP Server ) with ESMTPA ID 55b3ad14fada2636cb412d93e8430ef3; Wed, 24 Jan 2018 18:18:57 +0000 (UTC) Subject: Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev/... To: cem@freebsd.org Cc: src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org References: <201801211542.w0LFgbsp005980@repo.freebsd.org> <51ff8aef-5660-7857-e4d5-12cdc77bc071@FreeBSD.org> From: Pedro Giffuni Message-ID: <544fb5e8-b9b9-9f11-553f-4685b75387c7@FreeBSD.org> Date: Wed, 24 Jan 2018 13:18:56 -0500 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Jan 2018 18:19:03 -0000 On 01/24/18 12:37, Conrad Meyer wrote: > On Tue, Jan 23, 2018 at 11:40 AM, Pedro Giffuni wrote: >> On 23/01/2018 14:08, Conrad Meyer wrote: >>> On Sun, Jan 21, 2018 at 7:42 AM, Pedro F. Giffuni wrote: >>>> Author: pfg >>>> Date: Sun Jan 21 15:42:36 2018 >>>> New Revision: 328218 >>> I'm confused about this change. Wouldn't it be better to remove the >>> annotation/attributes from mallocarray() than to remove the protection >>> against overflow? >> >> Not in my opinion: it would be better to detect such overflows at compile >> time (or through a static analyzer) than to have late notification though >> panics. > Sure, it would be better, but some situations are only detected at > runtime -- hence mallocarray. And occasional use of the annotations > on systems with plenty of RAM would keep the source tree free of > compiler-detectable overflows, which I suspect are incredibly > uncommon. I think the runtime error cases are way more uncommon, specially if the checks are unnecessary. In any case I collected the patch with malloc--> mallocarray changes in PR 225197. Maybe their are useful with fuzzing. >> The blind use of mallocarray(9) is probably a mistake also: we >> shouldn't use it unless there is some real risk of overflow. > I'm not sure I follow that. You normally don't get "tainted" values in the kernel. Most of the allocations in question were variables with very small number multiplied by the size of an int. As Bruce mentioned, we don't do big allocations with malloc(9), at least not the size required to get an unsigned number overflow. In such cases checking for an overflow is a complete waste of time. And then there is also the bug of mallocarray(9) size types not matching malloc(9). >>> (If the compiler is fixed in the future to not use >>> excessive memory with these attributes, they can be conditionalized on >>> compiler version, of course.) >> All in all, the compiler is not provably wrong: it's just using more swap >> space, which is rather inconvenient for small platforms but not necessarily >> wrong. > Seems wrong if it's a noticeable amount. Maybe we could flip the > annotations on or off with a low-ram build knob or something like > that. There is actually no proof that this was related to the attributes. I absolutely dislike the idea of disabling the attributes and even more the idea of adding complex machinery to the system headers to account for such case. Pedro.