From owner-freebsd-arch@freebsd.org Mon Feb 1 23:24:56 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 4DB71A9776B for ; Mon, 1 Feb 2016 23:24:56 +0000 (UTC) (envelope-from cse.cem@gmail.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 2C7541E51 for ; Mon, 1 Feb 2016 23:24:56 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: by mailman.ysv.freebsd.org (Postfix) id 29237A9776A; Mon, 1 Feb 2016 23:24:56 +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 28BBEA97769 for ; Mon, 1 Feb 2016 23:24:56 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: from mail-qg0-f47.google.com (mail-qg0-f47.google.com [209.85.192.47]) (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 E0BED1E4F for ; Mon, 1 Feb 2016 23:24:55 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: by mail-qg0-f47.google.com with SMTP id o11so133670014qge.2 for ; Mon, 01 Feb 2016 15:24:55 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:reply-to:in-reply-to:references :date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=WUGwnb9oStHioOVVgtOoBkst6Z1nwBDs0girDJYaNoI=; b=Wnkd7rzy4nMBH4AgFNuPqbzhiv5avkBxvOJQJuziOxPmMNzXr/Iio5Iinb5qjmd0RO hScD3HIt/B7vFgYPa/WmyGRbT2wTtLuC62FQ9Odmpx0vKaubpWYoS5orZPW7+O6MQqU5 a1TfPbFi6bknzJvartjvkkaqjbl7naQsxyf2uk2VET52Rswc/kE8rVNHIlHxxr7iXPZ+ c7b/chxm7sCHp7PBVNFzpQvG/aq168q1urNoQwf91ogwCryCE/sndqT6S2RoaYtPV805 qCeUlNR+rwoWgar/6mtBOEwsGVV4j9POajPRWA1Vkd6lC4nxlqRW1WyZDJTpZL+p00SJ QOsg== X-Gm-Message-State: AG10YOR5qV52IQ1ZWovH9a3uB8etB+XH1laVETzhcrUnyqoofmmLhEYcr4Tu4IyXMoiE7w== X-Received: by 10.140.28.66 with SMTP id 60mr31424754qgy.74.1454367458018; Mon, 01 Feb 2016 14:57:38 -0800 (PST) Received: from mail-yk0-f171.google.com (mail-yk0-f171.google.com. [209.85.160.171]) by smtp.gmail.com with ESMTPSA id n203sm4957672qhc.43.2016.02.01.14.57.37 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 01 Feb 2016 14:57:37 -0800 (PST) Received: by mail-yk0-f171.google.com with SMTP id z13so65632599ykd.0 for ; Mon, 01 Feb 2016 14:57:37 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.37.64.198 with SMTP id n189mr6104480yba.124.1454367457260; Mon, 01 Feb 2016 14:57:37 -0800 (PST) Reply-To: cem@FreeBSD.org Received: by 10.37.2.132 with HTTP; Mon, 1 Feb 2016 14:57:37 -0800 (PST) In-Reply-To: References: <20160201210256.GA29188@yamori.belopuhov.com> <1EA0ECF5-D7AC-430E-957D-C4D49F9A872B@bsdimp.com> Date: Mon, 1 Feb 2016 14:57:37 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: OpenBSD mallocarray From: Conrad Meyer To: Warner Losh Cc: Mike Belopuhov , "freebsd-arch@freebsd.org" , Ryan Stone Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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:24:56 -0000 On Mon, Feb 1, 2016 at 2:49 PM, Warner Losh wrote: > On Mon, Feb 1, 2016 at 3:19 PM, Conrad Meyer wrote: >> On Mon, Feb 1, 2016 at 1:12 PM, Warner Losh wrote: >> > Yea, we don=E2=80=99t want it calling panic. Ever. That turns an overf= low >> > 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. Exactly. malloc requests that overflow a size_t *cannot* happen. Today they are undefined behavior / at best memory corruption. This is not something we want users to be able to trigger, ever. > If the user can trigger this > panic by passing some bogus, unchecked value that doesn't get checked > until here, that's bad. Agreed. > That's fundamentally different than getting > 'freeing free inode' from ufs. Users can't trigger the latter. Users must not be able to trigger this panic either. > We disagree on this then. This isn't a 'fail safe' this is a 'fail > with denial of system'. that' What is your alternative proposal, in this scenario, that results in any better result than DoS? Heap corruption and code execution is not a better result than DoS, IMO. Keep in mind that if the user controls array size allocation in this scenario, even without the panic they may DoS the system with a huge-but-smaller-than-size_t kernel memory allocation. >> 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. If we don't panic explicitly, we panic implicitly for unfixed M_WAITOK users of the interface. I think the explicit panic is better, at least until callers are fixed. > M_NOWAIT callers know to check for NULL, or they have a bug. Yes, but it's a different error. E.g. callers cannot handle EAGAIN like EI= NVAL. > 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. I agree CANFAIL is terrible but come down on the opposite side for the default behavior when given overflowing arguments. Overflowing arguments should never happen. It is a bogus use of the API. Best, Conrad