From owner-freebsd-arch@freebsd.org Mon Feb 1 23:01:15 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id AC246A76E5D for ; Mon, 1 Feb 2016 23:01:15 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mailman.ysv.freebsd.org (mailman.ysv.freebsd.org [IPv6:2001:1900:2254:206a::50:5]) by mx1.freebsd.org (Postfix) with ESMTP id 89EE11009 for ; Mon, 1 Feb 2016 23:01:15 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mailman.ysv.freebsd.org (Postfix) id 86E5DA76E5A; Mon, 1 Feb 2016 23:01:15 +0000 (UTC) Delivered-To: arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 857E5A76E57 for ; Mon, 1 Feb 2016 23:01:15 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-qg0-x234.google.com (mail-qg0-x234.google.com [IPv6:2607:f8b0:400d:c04::234]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 3810C1004 for ; Mon, 1 Feb 2016 23:01:15 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-qg0-x234.google.com with SMTP id 6so133075382qgy.1 for ; Mon, 01 Feb 2016 15:01:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=OrU5oez5BDzlrehKZkGmTDVcJ/cILZRct8TU+DjbzvE=; b=Gmf7qa4RrB/E+BPj1b55Gzve03cnxWaKnzNQKnTqnQD2gkLjFvkUOsq12zFQ53JGNU cK6McxUZjfRCWCRBJlJ7i/nwzM6Y1KE6mKbyavwCF//DYKpTuylEghrdjWRLmBAEEYXD uHyF8BP6cEF1zf601tMedcP6dvL0+4scHEuDXXpEeREQJInfhHmahDp6jR0Jvy5robzh DrFY5qtXP5TxTCPUdOEFAOC3BH9VI6V3ybP+W7pyCiUcyzMM9i41sPeyXtPMqcqG3c50 Ruu2Gv+8fA37pBSkeOcvZAgAT6trnRu8sybJUdsWjmrxO7jfQc/NfwhSouyCllKVcXkX 3Tcg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:sender:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=OrU5oez5BDzlrehKZkGmTDVcJ/cILZRct8TU+DjbzvE=; b=lutOoNDmZsg7MNkQbHmmT0MbMNpnffGV6o63IkA1rqSJ1ENIWfL/5CLJNWVoK3uwmP a7HFKcnNHXbWDjqVwv7WTl+IKMFQ1YGpm6aoT+FSn2lIzjPWeFVr//wjWrWjTYMQHE3+ ZsZ6R9qv5dubAvS8SfVlX8bR8SogxW8YtBw/uC0gvgnxWRxXpL5OYV0SkMT64GVOS1pv Vsj8qXarQoF+nvgMTMWzYzI56hVLFvQHEOt3EQW80qniz448ie1pV2maQduVO63ZUApl H5PDqKnMOM5AWhgoBRsn22w5LEjAMG/sAdK/YDaNdL9Oi5JFm2hTRQcb2491sRQqGgAw OkYA== X-Gm-Message-State: AG10YOSA4OdLuUtGA3SNmCXxunMLtf5uBO+XrzZfQlJci3rehwl4gE71D9L3B7eWpO1b1qvQ+uml7hww/5YmyA== MIME-Version: 1.0 X-Received: by 10.140.99.53 with SMTP id p50mr31336915qge.97.1454367674361; Mon, 01 Feb 2016 15:01:14 -0800 (PST) Sender: wlosh@bsdimp.com Received: by 10.140.30.166 with HTTP; Mon, 1 Feb 2016 15:01:14 -0800 (PST) X-Originating-IP: [69.53.245.11] In-Reply-To: <20160201224854.GB1747@spindle.one-eyed-alien.net> References: <20160201210256.GA29188@yamori.belopuhov.com> <1EA0ECF5-D7AC-430E-957D-C4D49F9A872B@bsdimp.com> <20160201224854.GB1747@spindle.one-eyed-alien.net> Date: Mon, 1 Feb 2016 16:01:14 -0700 X-Google-Sender-Auth: M8YEVlymbk--hESawu_aU7fUITM Message-ID: Subject: Re: OpenBSD mallocarray From: Warner Losh To: Brooks Davis Cc: Mike Belopuhov , "freebsd-arch@freebsd.org" , Ryan Stone Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.20 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Feb 2016 23:01:15 -0000 On Mon, Feb 1, 2016 at 3:48 PM, Brooks Davis wrote: > On Mon, Feb 01, 2016 at 02:12:20PM -0700, Warner Losh wrote: > > > > > On Feb 1, 2016, at 2:02 PM, Mike Belopuhov wrote: > > > > > > On Mon, Feb 01, 2016 at 15:56 -0500, Ryan Stone wrote: > > >> On Mon, Feb 1, 2016 at 3:16 PM, Conrad Meyer wrote: > > >> > > >>> > > >>> Sure. +1 from me. I don't think we want the M_CANFAIL hack, though. > > >>> > > >>> Best, > > >>> Conrad > > >>> > > >>> > > >> That may be the OpenBSD equivalent of M_NOWAIT. > > > > > > Not quite. From the man page: > > > > > > M_CANFAIL > > > > > > In the M_WAITOK case, if not enough memory is available, > > > return NULL instead of calling panic(9). If mallocarray() > > > detects an overflow or malloc() detects an excessive > > > allocation, return NULL instead of calling panic(9). > > > > Yea, we don???t want it calling panic. Ever. That turns an overflow > > into a DoS. Arguments should be properly checked so we can > > properly return EINVAL for bat-**** crazy ones. FreeBSD???s malloc > > doesn???t cave an excessive detector in it. > > > > My concern with this is that we have a number of different allocation > > routines in FreeBSD. This only goes after the malloc() vector, and > > even then it requires code changes. > > > > At best, CANFAIL is a kludge to fail with a panic instead of an > > overflow. That???s got to be at most a transient thing until all the > > code that it is kludged into with out proper thought is fixed. I???m not > > sure that???s something that we want to encourage. I???m all for > > safety, but this flag seems both unsafe and unwise. > > Given that current code could be doing literally anything in the > overflow case (and thanks to modern undefined behavior optimizations is > likely doing something extraordinarily bizarre), I think turning overflows > into panics is a good thing. Yes, you can argue that means you've added > a DoS vector, but best case you had an under allocation and bizarre > memory corruption before. If the default or even only behavior is going > to be that overflow fails then we need a static checker that ensure we > check the return value even in the M_WAITOK. Otherwise there will be > blind conversions of malloc to mallocarray that go unchecked. > Returning NULL should be sufficient. Blind conversion of malloc to mallocarray in the kernel is also stupid. Intelligent conversion is needed to ensure that the error conditions are handled correctly. There's no need for a flag to say 'I am going to do the right thing if you give me NULL back'. The conversion should do the right thing when you get NULL back. A quick survey of the current kernel shows that there's not very many that could be using user defined values, but does show a large number of places where we could use this API. I guess this comes down to 'why is it an unreasonable burden to test the return value in code that's converted?' We're already changing the code. If you absolutely must have a flag, I'd prefer M_CANPANIC or something that is also easy to add for the 'mindless' case that we can easily grep for so we know when we're removed all the stupid 'mindless' cases from the tree. Warner