Received: by 10.192.165.148 with SMTP id m20csp3625317imm; Mon, 7 May 2018 15:57:59 -0700 (PDT) X-Google-Smtp-Source: AB8JxZp/7jyX10KwxRboMMG2JywoCLs8AnwUg2Ra2Qg1aNT7682//yHuDqaCBHav9VFAVUfb172S X-Received: by 10.98.70.155 with SMTP id o27mr37471150pfi.124.1525733879775; Mon, 07 May 2018 15:57:59 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1525733879; cv=none; d=google.com; s=arc-20160816; b=P5Fc4dUxSTpqtdk7aT4OR/vG6rFm+kGAP4YvP3AEtEoFD9JAsUwgf6+Zs6zcIbJWaN RQoSaIYU4hJMitCGAYuvxxKMnrBMoBrAU5eMBcujob6DhWTVdRVfj/EVh3ZVqUPrnUf6 /rg/9gb6aoXvVLcfGe8cuQSpPZfBl3Jw6k5kQd+pg7wVTIdxiyVQmvLf0K6fx8+7bhr1 TJB1eMmM2VeO/odD0nynpEK+lO0wWKmXXhdil1zeSUqEtUa/bYDgI6BvobnElGVbBbfL 1RqY1n5EgB31bLQb6evKmVTtu2fTTjBzbSz8UR7NqC47cIkeUDFBnDTJ0I40ikYKpKI/ iDrw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :references:in-reply-to:mime-version:dkim-signature :arc-authentication-results; bh=cTXWHHZeyR6TCbxqaWgAv97+yepwhDaMk3KKt2scu7E=; b=pb0SnVZX6PnEL6yeYxxfSjEl38P8PZ3zAAvIQIymytUjO01MqXfCmZ1rAII96dv8Eh 78V4z4LrGbjo2s8UWhyiaNOr6wzwJ4koFOvupognbPqbWWQ7y0ClPS36fh5rja/VEq4w madF0BjucBLQDSeFIyuIk01jtiT5Hp1w90y8sR15fa6wHxbU7uVICmpcsQdenzOd6AL+ YhJhlY1vDg5WpTvC2VxcxzPqiecEfHPLJSXmP+6dF2FsKq2Vf/ZQiQW6MIA5BDjMd3sv XR/b3HkUv0ga5+kpMY+r5RwuykNPYDoT1Wx8oHIoGNRuRcda5K7JYfsOpXCoDQ+VYIwH FJ1g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=EDfkEKsq; 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; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id b10-v6si22780579plb.177.2018.05.07.15.57.45; Mon, 07 May 2018 15:57:59 -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=pass header.i=@google.com header.s=20161025 header.b=EDfkEKsq; 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; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752954AbeEGW41 (ORCPT + 99 others); Mon, 7 May 2018 18:56:27 -0400 Received: from mail-ua0-f177.google.com ([209.85.217.177]:37134 "EHLO mail-ua0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752158AbeEGW40 (ORCPT ); Mon, 7 May 2018 18:56:26 -0400 Received: by mail-ua0-f177.google.com with SMTP id i3so19460542uad.4 for ; Mon, 07 May 2018 15:56:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=cTXWHHZeyR6TCbxqaWgAv97+yepwhDaMk3KKt2scu7E=; b=EDfkEKsqXzfI/obnEJy7+7EbHQIpM5Zwgmi0TRmW0lFjF4wuVYcc8+rOZAcu4p8XYk t2mIBM8aTGu3G5V+08F4JEu6/7cEInLQdvxFs97TQgNXLarRsjC6n3JYUJNXDuuFoadl BllA3HKe4lFexbThi5FqNTGScgTo8N74L2++7Huj5dPAUksD3AETws+ZZb2BLERhrcjX lgAES3k65cVEaywPJDFtwFWJHSRGzHyVYihj77iPBMkyId82PdezhvfOBabFtQRSeLhl YqwElJoovm2eLwvhkEEzF2MPpjPWdvVqUmAsFf9bPBuWNrrP1Fc4dwyNNKAd0fgRTRUq 3LrA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=cTXWHHZeyR6TCbxqaWgAv97+yepwhDaMk3KKt2scu7E=; b=MJw+FOJy4rTQ6oyKwWZ04PH7h8EtysjzPCF09sTkDa7lywZklfFh2Wie/XEslioxld uJ6Lk7m4H9pJUHBiL+BjTZR5LGxiQJ8vdXFcNBFezUcbK3sgyzKgUUe4bUQ9rm0zYxVB O2MRh8NHahevglcJb7ECTrpYTZ/BIeJzWmMfJrpO38hwroxlMbGFC3bAtiw1IOFY7Nxu zoUyJHoXu8E7ibXa9DWr4swQsd6fAJTnbup8w8VBUcosHnHLaZ2m2awk6Pumj9Vlme1F /rmQHPorDmNcCWiY3aJCcQxEoKSQVrXgAHJdwt2poegyp6CsgZOHgwJUlT3dxllrN+8/ xiCw== X-Gm-Message-State: ALQs6tD5CZonzFnmDn69XHYvqOZBwd6nLhNRDobyWRFnTwPPJRnqluO8 1ShRuC96k3eAaciCQD5jpGG544PEEXVPHf2MfurThw== X-Received: by 10.176.84.78 with SMTP id o14mr33362333uaa.164.1525733785824; Mon, 07 May 2018 15:56:25 -0700 (PDT) MIME-Version: 1.0 Received: by 10.31.11.209 with HTTP; Mon, 7 May 2018 15:56:25 -0700 (PDT) In-Reply-To: References: <20180505034646.GA20495@bombadil.infradead.org> From: Kees Cook Date: Mon, 7 May 2018 15:56:25 -0700 Message-ID: Subject: Re: *alloc API changes To: Rasmus Villemoes Cc: Matthew Wilcox , Matthew Wilcox , Linux-MM , LKML Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, May 7, 2018 at 2:41 PM, Rasmus Villemoes wrote: > If instead we do > > static inline void *kmalloc_array(size_t n, size_t size, gfp_t flags) > { > size_t p; > if (check_mul_overflow(n, size, &p)) > return NULL; > return __kmalloc(p, flags); > } > > we'd not get any extra code in the caller (that is, just a "mul" and > "jo", instead of a load-immediate, mul, cmov), because gcc should be > smart enough to combine the "return NULL" with the test for NULL which > the caller code has, and thus make the jump go directly to the error > handling (that error handling is likely itself a jump, but the "jo" will > just get redirected to the target of that one). > > Also, I'd hate to have sat_mul not really saturating to type_max(t), but > some large-enough-that-all-underlying-allocators-reject-it. Yeah, this continues to worry me too. > All allocators still need to reject insane sizes, since those can happen > without coming from a multiplication. So sure, some early size > > MAX_SANE_SIZE check in the out-of-line functions should be rather cheap, > and they most likely already exist in some form. But we don't _have_ to > go out of our way to make the multiplication overflow handling depend on > those. > >> >> 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); > > Urgh. Do you want to get completely rid of kmalloc_array() and move the > mul() into the call-sites? That obviously necessitates mul returning a > big-enough sentinel. I'd hate that. Not just because of the code-gen, > but also because of the problem with giving mul() sane semantics that > still make it immune to the extra arithmetic that will inevitably be > done. There's also the problem with foo and bar having different, > possibly signed, types - how should mul() handle those? A nice benefit > from having the static inline wrappers taking size_t is that a negative > value gets converted to a huge positive value, and then the whole thing > overflows. Sure, you can build that into mul() (maybe make that itself a > static inline), but then it doesn't really deserve that generic name > anymore. If the struct_size(), array_size(), and array3_size() all take size_t, then I think we gain the same protection, yes? i.e. they are built out of your overflow checking routines. Even though I'm nervous about SIZE_MAX wrapping on other uses, I do agree that doing early rejection in the allocators makes a lot more sense. I think we can have bot the SIZE_MAX early-abort of "something went very wrong", and the internal "I refuse to store that much" that is per-allocator-tuned. >> kmalloc(foo * bar, GFP_KERNEL | __GFP_ZERO); >> into >> kzalloc(foo * bar, GFP_KERNEL); >> into >> kcalloc(foo, bar, GFP_KERNEL); >> into >> kzalloc(mul(foo, bar), GFP_KERNEL); > > Yeah, part of the API mess is just copied from C (malloc vs calloc). We > could make it a bit less messy by calling it kzalloc_array, but we have > 1700 callers of kcalloc(), so... Coccinelle can fix those callers. ;) But we need to make sure we're still generating sane machine code before we do that... -Kees -- Kees Cook Pixel Security