Received: by 10.192.165.148 with SMTP id m20csp3496152imm; Mon, 7 May 2018 13:20:54 -0700 (PDT) X-Google-Smtp-Source: AB8JxZoVO1uMVyTMq/H5OViDSh+5pYbckUsxth0U8pif4ZOEQ2l9t+gLpLxK6mW+hXK+yRRnyIKe X-Received: by 2002:a17:902:848e:: with SMTP id c14-v6mr35568501plo.129.1525724454423; Mon, 07 May 2018 13:20:54 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1525724454; cv=none; d=google.com; s=arc-20160816; b=CfvCeJOp2LaO3rzcwdMkTaJh6zMuk+jH3cEdOk2AoZ9ORQmmMa5gyt9MiX/xA9z8Ps qrWEgRS4wGMz+/x46LONaE/xZt/itVSzO3xLPFOE4vAgblXHHvrOno3KTC1CzWsj/sDn uT3A16vywqGnD8w467vWlGxZnw90OGgv+bRwDFDVrqjzv7T26mKBWGJONoYdmOT6aeDC 6IiZeUxMuhHwNfsbqIYQTSuJF6NaTwWc4H02cjjrakVYtNLGFNn7MPHShruu5+uljfcm Pi2Tw9k+vT9989irBWs1XrwZHxXX47kuNh2+MoAkn49avK1VPCSrj0BRn7RIT6fiBWM7 5Bng== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:dkim-signature:arc-authentication-results; bh=JDUpwmlnG/9w4YDa6SusQZNR8M2tl48eUtp/qqPoDJY=; b=Qe1jPpsky0t/IfM/HzOvCOzhcoC0VbHHZ3aIS6ujjE51T2XQRN/qI47mLqBORzRdad EHZxqnSdI17f6ec/GuO8Z/l/fgZSpukek1Gvd5sHgjAbVArEV/IK3X6A+ZdXYk1xlBkU Kdb2oBA6hsMPHjCjZ5PI35foMv/zJ89hBQlka2BbmtYbfQbjIAjrebi5CjdQSuPoVOy2 cAvsBZF3+hxQJcK8rHPeKQyVwxlKZSc7MRdEeJamH6esskhYYGMl7jjjpVdukF+pk/9A IYP1AgCR/ogdkR3C/tUBK2xgSlBcowqvgzqpl8SlFCp0YCmAKnuNBw380TWvxB9hGaBE 5Omg== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@infradead.org header.s=bombadil.20170209 header.b=e3qikUCx; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id v4-v6si22566722ply.351.2018.05.07.13.20.39; Mon, 07 May 2018 13:20:54 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=fail header.i=@infradead.org header.s=bombadil.20170209 header.b=e3qikUCx; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752960AbeEGUTt (ORCPT + 99 others); Mon, 7 May 2018 16:19:49 -0400 Received: from bombadil.infradead.org ([198.137.202.133]:56166 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752923AbeEGUTr (ORCPT ); Mon, 7 May 2018 16:19:47 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20170209; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=JDUpwmlnG/9w4YDa6SusQZNR8M2tl48eUtp/qqPoDJY=; b=e3qikUCxS+XGantwjzlg67/7n FpRQ/dcOoLFvVci+i2Sl3Brsl86s7jnwTz4ocj4kcNbgYo84hyO0N0v7wxOomo/arAK07mSXKfyVD v5/jl8nVrPR8GK5hxFkZyQZ5ah1yHqnSj12hx8osaSKm7wPQwLpVUrueHGxS9DplPH+SVD01bXkE6 btm87c/rk7uSTKXEVQBI+9qZ+UuD1FwI6JRJxhVYQstzbrtmoljmOVDBC8L+u0xGeS9l/krJZ7n0U Rh4mwrehbRgdBJqWPbw3KttFGrf5H+Tajlk0LfUsdbeSWTYHa1TOg+MxoWngL9zi6RZmNiSF6Z1pw G4ZEnSyrw==; Received: from willy by bombadil.infradead.org with local (Exim 4.90_1 #2 (Red Hat Linux)) id 1fFmbp-0008EF-Es; Mon, 07 May 2018 20:19:45 +0000 Date: Mon, 7 May 2018 13:19:45 -0700 From: Matthew Wilcox To: Kees Cook Cc: Matthew Wilcox , Linux-MM , LKML , Rasmus Villemoes Subject: Re: *alloc API changes Message-ID: <20180507201945.GB15604@bombadil.infradead.org> References: <20180505034646.GA20495@bombadil.infradead.org> <20180507113902.GC18116@bombadil.infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.2 (2017-12-15) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, May 07, 2018 at 09:03:54AM -0700, Kees Cook wrote: > On Mon, May 7, 2018 at 4:39 AM, Matthew Wilcox wrote: > > On Fri, May 04, 2018 at 09:24:56PM -0700, Kees Cook wrote: > >> On Fri, May 4, 2018 at 8:46 PM, Matthew Wilcox wrote: > >> The only fear I have with the saturating helpers is that we'll end up > >> using them in places that don't recognize SIZE_MAX. Like, say: > >> > >> size = mul(a, b) + 1; > >> > >> then *poof* size == 0. Now, I'd hope that code would use add(mul(a, > >> b), 1), but still... it makes me nervous. > > > > That's reasonable. So let's add: > > > > #define ALLOC_TOO_BIG (PAGE_SIZE << MAX_ORDER) > > > > (there's a presumably somewhat obsolete CONFIG_FORCE_MAX_ZONEORDER on some > > architectures which allows people to configure MAX_ORDER all the way up > > to 64. That config option needs to go away, or at least be limited to > > a much lower value). > > > > On x86, that's 4k << 11 = 8MB. On PPC, that might be 64k << 9 == 32MB. > > Those values should be relatively immune to further arithmetic causing > > an additional overflow. > > But we can do larger than 8MB allocations with vmalloc, can't we? Yes. And today with kvmalloc. However, I proposed to Linus that kvmalloc() shouldn't allow it -- we should have kvmalloc_large() which would, but kvmalloc wouldn't. He liked that idea, so I'm going with it. There are very, very few places which should need kvmalloc_large. That's one million 8-byte pointers. If you need more than that inside the kernel, you're doing something really damn weird and should do something that looks obviously different. > > I don't think it should go in the callers though ... where it goes in > > the allocator is up to the allocator maintainers ;-) > > We need a self-test regardless, so checking that each allocator > returns NULL with the saturated value can be done. Yes, absolutely. > >> > I'd rather have a mul_ab(), mul_abc(), mul_ab_add_c(), etc. than nest > >> > calls to mult(). > >> > >> Agreed. I think having exactly those would cover almost everything, > >> and the two places where a 4-factor product is needed could just nest > >> them. (bikeshed: the very common mul_ab() should just be mul(), IMO.) > >> > >> > Nono, Linus had the better proposal, struct_size(p, member, n). > >> > >> Oh, yes! I totally missed that in the threads. > > > > so we're agreed on struct_size(). I think rather than the explicit 'mul', > > perhaps we should have array_size() and array3_size(). > > I do like the symmetry there. My earlier "what if someone does +1" > continues to scratch at my brain, though I think it's likely > unimportant: there's no indication (in the name) that these calls > saturate. Will someone ever do something crazy like: array_size(a, b) > / array_size(c, d) and they can, effectively, a truncated value (if > "a, b" saturated and "c, d" didn't...)? Without CPU support for a saturated value, there's no "safe" saturated value. You can always come up with some arithmetic that will bring it back into the valid range. All we can do is go "large enough" and hope. > >> Right, no. I think if we can ditch *calloc() and _array() by using > >> saturating helpers, we'll have the API in a much better form: > >> > >> kmalloc(foo * bar, GFP_KERNEL); > >> into > >> kmalloc_array(foo, bar, GFP_KERNEL); > >> into > >> kmalloc(mul(foo, bar), GFP_KERNEL); > > > > kmalloc(array_size(foo, bar), GFP_KERNEL); > > I can't come up with a better name. :P When it was "mul()" I was > thinking "smul()" for "saturating multiply". sarray_size() seems ... > bonkers. smul would mean 'signed multiply' to me, having read the GCC manual: -- Built-in Function: bool __builtin_smul_overflow (int a, int b, int *res) but I thought of another problem with array_size. We already have ARRAY_SIZE and it means "the number of elements in the array". so ... struct_bytes(), array_bytes(), array3_bytes()? > > I think we're broadly in agreement here! > > Do we want addition helpers? (And division and subtraction?) Keeping our focus on allocations ... do we have plain additions (as opposed to multiply-and-add?) And subtraction?