From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B9B3CC433EF for ; Fri, 10 Sep 2021 18:43:55 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 511476124D for ; Fri, 10 Sep 2021 18:43:55 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 511476124D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 9AB826B0071; Fri, 10 Sep 2021 14:43:54 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 95BBC6B0072; Fri, 10 Sep 2021 14:43:54 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7FC1A6B0073; Fri, 10 Sep 2021 14:43:54 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0065.hostedemail.com [216.40.44.65]) by kanga.kvack.org (Postfix) with ESMTP id 6FD326B0071 for ; Fri, 10 Sep 2021 14:43:54 -0400 (EDT) Received: from smtpin10.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 05CF83A2DA for ; Fri, 10 Sep 2021 18:43:54 +0000 (UTC) X-FDA: 78572537988.10.07BCC7E Received: from mail-pj1-f53.google.com (mail-pj1-f53.google.com [209.85.216.53]) by imf24.hostedemail.com (Postfix) with ESMTP id B1C24B00009F for ; Fri, 10 Sep 2021 18:43:53 +0000 (UTC) Received: by mail-pj1-f53.google.com with SMTP id j10-20020a17090a94ca00b00181f17b7ef7so2146588pjw.2 for ; Fri, 10 Sep 2021 11:43:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Lv1p9zHba3I9cECEqRtPSSNa5kOoo9emyH9mJ0H8m8o=; b=bjpP9QHFJ+pYcG5i9iwcHyxO5hzHyEzm3e1HqmIFsMPrh1GMlTxAZZXh76EB9/QG8R OjsRzXncN5A0i4Jdvnr+jqWJdk2cvvOYocddzJX+dphXM0GINC3lcoYBg2X38mSgaBCE U0WuKIw0RI9MsgjqW4cfe/hDtC1CTCC9apdYc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=Lv1p9zHba3I9cECEqRtPSSNa5kOoo9emyH9mJ0H8m8o=; b=f/aiuFaC8qlg5MZdw+6/A7ot4owhlF0BUF/8TuonE3U8r5WhxpMLYrzUY5tqwMTGAL KHIST/pYFa1uP1zqM0cpeqcTJKRQbfjyg/ynSe8mr76s3qzm3H/Qx6q/jyegjv6qqW3p Fr8s+G/NhMPfrfW7+p0BoXJ5BXwibz5CN04bepsvwehXKLfSS2MxbFfJ2lXLRsuxOrz1 MCE9SykOX5DdW+VCOK9trUiCWdntwabtJLtz2z2ERsBroaLgLdhza1dI9arnTSQGgMiD u5cQX2TwkX7y42XAij5WyJnsfnR5QZVaN4gUNJ4l4lcju3dYuTiHMdjQLVJe1vyb8eBi rlmA== X-Gm-Message-State: AOAM532cXeT9SVw6E1ynRRGQnTR3UjF01jMs0zI3OASXcHN/GHy6XA5U 2EmJniCJRNfuitlaS795vnoS2Q== X-Google-Smtp-Source: ABdhPJzbR+ovIpV5OqJJYBmG2oeAsqbsKVwHDcgn5Kv05lSErl4vHRfRj8TTEAcF6HmnRDc7ke7YXQ== X-Received: by 2002:a17:90a:8b98:: with SMTP id z24mr11135526pjn.87.1631299432723; Fri, 10 Sep 2021 11:43:52 -0700 (PDT) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id c4sm5776533pji.51.2021.09.10.11.43.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Sep 2021 11:43:52 -0700 (PDT) Date: Fri, 10 Sep 2021 11:43:51 -0700 From: Kees Cook To: Linus Torvalds Cc: Andrew Morton , apw@canonical.com, Christoph Lameter , Daniel Micay , Dennis Zhou , dwaipayanray1@gmail.com, Joonsoo Kim , Joe Perches , Linux-MM , Lukas Bulwahn , mm-commits@vger.kernel.org, Nathan Chancellor , Nick Desaulniers , Miguel Ojeda , Pekka Enberg , David Rientjes , Tejun Heo , Vlastimil Babka Subject: Re: [patch 9/9] mm/vmalloc: add __alloc_size attributes for better bounds checking Message-ID: <202109101138.53FCADF5C@keescook> References: <20210909200948.090d4e213ca34b5ad1325a7e@linux-foundation.org> <20210910031046.G76dQvPhV%akpm@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: B1C24B00009F Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=bjpP9QHF; dmarc=pass (policy=none) header.from=chromium.org; spf=pass (imf24.hostedemail.com: domain of keescook@chromium.org designates 209.85.216.53 as permitted sender) smtp.mailfrom=keescook@chromium.org X-Stat-Signature: a9zepdxfs7ge1yytzzy3srmfzepqocb8 X-HE-Tag: 1631299433-128450 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, Sep 10, 2021 at 10:23:48AM -0700, Linus Torvalds wrote: > On Thu, Sep 9, 2021 at 8:10 PM Andrew Morton wrote: > > > > +__alloc_size(1) > > extern void *vmalloc(unsigned long size); > [...] > > All of these are added in the wrong place - inconsistent with the very > compiler documentation the patches add. > > The function attributes are generally added _after_ the function, > although admittedly we've been quite confused here before. > > But the very compiler documentation you point to in the patch that > adds these macros gives that as the examples both for gcc and clang: > > + * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alloc_005fsize-function-attribute > + * clang: https://clang.llvm.org/docs/AttributeReference.html#alloc-size > > and honestly I think that is the preferred format because this is > about the *function*, not about the return type. > > Do both placements work? Yes. > > Have we been confused about this before? Yes. I note that our __printf > attributes in particular have been added in odd places. And our > existing __malloc annotations seem to correct in and > but then randomly applied in some other places. > > I really think it's pointlessly stupid and hard to read/grep for to > make it be a separate line before the whole thing. This was bike-shed on the list, and this result seemed to be consensus, but I kind of dislike all the options. Either things are on separate lines or they're trailing attributes that get really long, etc. Ugh. I'm happy to clean all of it up into whatever form can be agreed on for the "correct" placement. > I also think it should have taken over the "__malloc" name that is > almost unused right now. Because why would you ever have > __alloc_size() without having __malloc(). I had originally set out to do that, but the problem with merging with __malloc is the bit in the docs about "and that the memory has undefined content". So we can't do that for kmalloc() in the face of GFP_ZERO, as well as a bunch of other helpers. I always get suspicious about "this will improve optimization because we depend on claiming something is 'undefined'". :| -Kees -- Kees Cook