From owner-freebsd-arch@freebsd.org Mon Feb 1 22:49: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 A9FB9A76AA4 for ; Mon, 1 Feb 2016 22:49: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 8721FBD4 for ; Mon, 1 Feb 2016 22:49:15 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mailman.ysv.freebsd.org (Postfix) id 8634DA76AA3; Mon, 1 Feb 2016 22:49: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 6BEC2A76AA2 for ; Mon, 1 Feb 2016 22:49:15 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-qg0-x22c.google.com (mail-qg0-x22c.google.com [IPv6:2607:f8b0:400d:c04::22c]) (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 2AE06BD0 for ; Mon, 1 Feb 2016 22:49:15 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-qg0-x22c.google.com with SMTP id e32so133856424qgf.3 for ; Mon, 01 Feb 2016 14:49: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=rG0tATVzdVStVwa66H0C/D70vGa4mPk9e8x6mibKmok=; b=RHem74BTnBuc1/Ws+wb60zysrBnyBuYGwQ4Am4vKvD+A83hyFr2AIW/nEsK0Zld9Vh H7CMDoFt5wWhgmIosSAJKpUN6dU/LNiXsffxJQw47LbkbFhvJmhCr6VVdbRE8z76jq3p XzZVpNptbBILJKItGN2OHJa8nepAy7hw8o4HffYaAgSV2g1CsWdy7CTDcMugziHUNEyb DKKgMf8RTh2MlT305tsOvorXLnMfXvQxGTaN+V0xRJ6uY5WBOB8LBv9EDsTjk6Th1ny0 kxoBvqytBQteNR899cloa77Q5tG1JWFRTJ+fG9oTlJUQbl0QPGvlTrxGb3snhxX0zmZB vrAw== 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=rG0tATVzdVStVwa66H0C/D70vGa4mPk9e8x6mibKmok=; b=SKt25/U0gxZXEV5xlWL3Caz+3zwoxPfMPT6c5mMx2cFKsO5dOtt1r5N7hjLbX/nJX0 QC+PhhoBTHsair2fnFiw5+K00v6n61uUdNbp206mrS2nd8G0X/sR/BgdIOzKxnlGRL/C d/8oNYVvFI1XRp/fyStup9SI7feWXRE5asIobvcPGZxPYi3SQdDinOWKSunqFn4K1f8s fyj3VKTRsi6iezQl89J0uQDiKLlV7QdKaM1MQPzFmUxvmq2+BSzTu4a5ihVQlZvu7DNn nB+9aJa8rGsxeK5AY5/sZ5TueSQZ3VGTykcGxrv/mpYRjBZ1sDKWxc8ICp/r0EqKf4AO t88w== X-Gm-Message-State: AG10YORITBsoy5qi2V3fNXa8CTvcbB3LgwEw0IllrrKLe4dIpjVdxZki4ZmcgZAquutXugqu+fwpcqrjPMaJfg== MIME-Version: 1.0 X-Received: by 10.140.229.209 with SMTP id z200mr15870589qhb.40.1454366954194; Mon, 01 Feb 2016 14:49:14 -0800 (PST) Sender: wlosh@bsdimp.com Received: by 10.140.30.166 with HTTP; Mon, 1 Feb 2016 14:49:14 -0800 (PST) X-Originating-IP: [69.53.245.11] In-Reply-To: References: <20160201210256.GA29188@yamori.belopuhov.com> <1EA0ECF5-D7AC-430E-957D-C4D49F9A872B@bsdimp.com> Date: Mon, 1 Feb 2016 15:49:14 -0700 X-Google-Sender-Auth: ybhq0qN_BYbnKYVn7_A7IAMf3xQ Message-ID: Subject: Re: OpenBSD mallocarray From: Warner Losh To: cem@freebsd.org Cc: Mike Belopuhov , "freebsd-arch@freebsd.org" , Ryan Stone Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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 22:49:15 -0000 On Mon, Feb 1, 2016 at 3:19 PM, Conrad Meyer wrote: > On Mon, Feb 1, 2016 at 1:12 PM, Warner Losh wrote: > > > >> On Feb 1, 2016, at 2:02 PM, Mike Belopuhov wrote: > >> 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=E2=80=99t want it calling panic. Ever. That turns an overfl= ow > > into a DoS. > > I disagree. The panic is essentially an assertion that malloc was > passed valid arguments. We have similar invariants assertions > throughout the kernel and it is the only sane thing to do with > overflow + M_WAITOK. M_WAITOK callers today will do something equally > stupid if they get a NULL result from mallocarray(). We only do this for things that can't happen. If the user can trigger this panic by passing some bogus, unchecked value that doesn't get checked until here, that's bad. That's fundamentally different than getting 'freeing free inode' from ufs. Users can't trigger the latter. > > > Arguments should be properly checked > > Yes! That's why the assertion is a good thing. We disagree on this then. This isn't a 'fail safe' this is a 'fail with denial of system'. that' > > At best, CANFAIL is a kludge to fail with a panic instead of an > > overflow. > > No, that's backwards. In CANFAIL mode, mallocarray returns NULL > instead of panicing immediately. It's a kludge so the caller doesn't > have to do overflow checking. Then it's a horrible interface. We failed badly with the MPSAFE interface. It was OK at first, but then when everybody uses it, then it because obvious that it was the wrong decision. > > That=E2=80=99s got to be at most a transient thing until all the > > code that it is kludged into with out proper thought is fixed. > > You mean the panic? What fallback behavior would you prefer? If the > caller requested an overflowing allocation, there really isn't > anything sane to do besides immediately panic (again, for M_WAITOK). > Even M_NOWAIT callers may or may not do something sane. > I'd prefer that we return NULL. Let the caller decide the policy, not some stupid thing down in the bowls of malloc because I forgot to include some stupid new flag. M_NOWAIT callers know to check for NULL, or they have a bug. It would be the same for mallocarray: you must check for NULL and unwind if you get that back. There's no need for the CANFAIL flag and it's a horrible API. Warner