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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 55CE0C433EF for ; Wed, 6 Oct 2021 04:52:52 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id CE8F2611CA for ; Wed, 6 Oct 2021 04:52:51 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org CE8F2611CA Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 11B926B006C; Wed, 6 Oct 2021 00:52:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0CB876B0071; Wed, 6 Oct 2021 00:52:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F23D46B0073; Wed, 6 Oct 2021 00:52:50 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0210.hostedemail.com [216.40.44.210]) by kanga.kvack.org (Postfix) with ESMTP id DF0646B006C for ; Wed, 6 Oct 2021 00:52:50 -0400 (EDT) Received: from smtpin17.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 98F2C3262C for ; Wed, 6 Oct 2021 04:52:50 +0000 (UTC) X-FDA: 78664792500.17.FC8DADC Received: from mail-lf1-f54.google.com (mail-lf1-f54.google.com [209.85.167.54]) by imf02.hostedemail.com (Postfix) with ESMTP id 472EF7001731 for ; Wed, 6 Oct 2021 04:52:50 +0000 (UTC) Received: by mail-lf1-f54.google.com with SMTP id b20so5099475lfv.3 for ; Tue, 05 Oct 2021 21:52:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=3KNLEP17J4fGFGePafL9e/em5qLyI4/H75ODeeqFPy4=; b=CaOWAOrwZbSOE5dR3MqwfrmPYlqUVOLLG6p1aODud/NwXWJQiRS5FhtR2EQZX8kY5m 7Ecehb4y6Skh2dqABZH6BcO1ivS01STlLtgU6rYr4X8l9ZdhQsrxdzKCZYqiQxsX04n4 RQx6CMFvAj8EzVHPEuI2uPckqFLkFZg5NV4y7hbH0ArIIQ6UZKmh/wBXA1CUlF6ZmWKw CeSu9cAiBP0WlSrHFkrisOU97HOwvbdx6OnjCEX6rSrIQVDmZn5LXFYvwtopuDuJlJTR 3OB1sBumZXVU1owfxavZ008gv6hlsiHPcCMfRiGz3S15Jnf+obS6yrzmoiLKvZCwB/TM xhPg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=3KNLEP17J4fGFGePafL9e/em5qLyI4/H75ODeeqFPy4=; b=apW4GukxvcGXL0vA/QP4tQEQmoZhH6FIVi+EBs3+0nz1PiaVe51BnX4wXIQx7xZH88 34AyA6vBb/WKPirikGr6lQHjvTt4LueFtzGUcclcaWE2QSCts+p0+sbrR9+2EKAb3844 ApNQxb9xddu8RIi8K37bkHlfvbi6XlzJLk5IghDj3J/F/xnbtHSzKF1gJhTojbvvbITh CTmXL3Fawqj0aDZ71HO6VrsRjxDI16iwP7BgvQJ6XmZKWRKhhfBT2hmSOmkKfvInPGiQ bOkmnInxsphXjaraaTc0BMZR9j07Ma0U7lNH0DvA2LOBW575InB0VsnNOsJ0eOG4nCg0 reqA== X-Gm-Message-State: AOAM530mbOaaJvdhCr1/jXA1GelT7gUOd1Nwv2eZPuSrrez2SFdIVFCz S3B4T1S3mLMhlwlZ10bGW9CYJrf1/GCm6I+G+Hzraw== X-Google-Smtp-Source: ABdhPJxpUZzFJ/6ioAhBGcD9vzEImJPunmpEp6cj/Rk2Cc1xOWjoRZ7tWmMKzDF3Pm9+uwpJa5b8o2ZEyoA6j2g2vus= X-Received: by 2002:ac2:4f01:: with SMTP id k1mr7561021lfr.157.1633495968423; Tue, 05 Oct 2021 21:52:48 -0700 (PDT) MIME-Version: 1.0 References: <20210930222704.2631604-1-keescook@chromium.org> <20210930222704.2631604-5-keescook@chromium.org> <20211005184717.65c6d8eb39350395e387b71f@linux-foundation.org> <202110052002.34E998B@keescook> <202110052056.F09CD8A@keescook> In-Reply-To: <202110052056.F09CD8A@keescook> From: Jann Horn Date: Wed, 6 Oct 2021 06:52:21 +0200 Message-ID: Subject: Re: [PATCH v3 4/8] slab: Add __alloc_size attributes for better bounds checking To: Kees Cook Cc: Andrew Morton , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Vlastimil Babka , Andy Whitcroft , Dennis Zhou , Dwaipayan Ray , Joe Perches , Lukas Bulwahn , Miguel Ojeda , Nathan Chancellor , Tejun Heo , Daniel Micay , Nick Desaulniers , Masahiro Yamada , Michal Marek , clang-built-linux@googlegroups.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-kbuild@vger.kernel.org, linux-hardening@vger.kernel.org Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 472EF7001731 X-Stat-Signature: 1tpo5qjz3xn8iys48q7bjiuc431xzdnf Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=CaOWAOrw; spf=pass (imf02.hostedemail.com: domain of jannh@google.com designates 209.85.167.54 as permitted sender) smtp.mailfrom=jannh@google.com; dmarc=pass (policy=reject) header.from=google.com X-Rspamd-Server: rspam06 X-HE-Tag: 1633495970-162562 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 Wed, Oct 6, 2021 at 5:56 AM Kees Cook wrote: > On Wed, Oct 06, 2021 at 05:22:06AM +0200, Jann Horn wrote: > > On Wed, Oct 6, 2021 at 5:06 AM Kees Cook wrote: > > > On Tue, Oct 05, 2021 at 06:47:17PM -0700, Andrew Morton wrote: > > > > On Thu, 30 Sep 2021 15:27:00 -0700 Kees Cook wrote: > > > > > > > > > As already done in GrapheneOS, add the __alloc_size attribute for regular > > > > > kmalloc interfaces, to provide additional hinting for better bounds > > > > > checking, assisting CONFIG_FORTIFY_SOURCE and other compiler > > > > > optimizations. > > > > > > > > x86_64 allmodconfig: > > > > > > What compiler and version? > > > > > > > > > > > In file included from ./arch/x86/include/asm/preempt.h:7, > > > > from ./include/linux/preempt.h:78, > > > > from ./include/linux/spinlock.h:55, > > > > from ./include/linux/mmzone.h:8, > > > > from ./include/linux/gfp.h:6, > > > > from ./include/linux/mm.h:10, > > > > from ./include/linux/mman.h:5, > > > > from lib/test_kasan_module.c:10: > > > > In function 'check_copy_size', > > > > inlined from 'copy_user_test' at ./include/linux/uaccess.h:191:6: > > > > ./include/linux/thread_info.h:213:4: error: call to '__bad_copy_to' declared with attribute error: copy destination size is too small > > > > 213 | __bad_copy_to(); > > > > | ^~~~~~~~~~~~~~~ > > > > In function 'check_copy_size', > > > > inlined from 'copy_user_test' at ./include/linux/uaccess.h:199:6: > > > > ./include/linux/thread_info.h:211:4: error: call to '__bad_copy_from' declared with attribute error: copy source size is too small > > > > 211 | __bad_copy_from(); > > > > | ^~~~~~~~~~~~~~~~~ > > > > make[1]: *** [lib/test_kasan_module.o] Error 1 > > > > make: *** [lib] Error 2 > > > > > > Hah, yes, it caught an intentionally bad copy. This may bypass the > > > check, as I've had to do in LKDTM before. I will test... > > > > > > diff --git a/lib/test_kasan_module.c b/lib/test_kasan_module.c > > > index 7ebf433edef3..9fb2fb2937da 100644 > > > --- a/lib/test_kasan_module.c > > > +++ b/lib/test_kasan_module.c > > > @@ -19,7 +19,12 @@ static noinline void __init copy_user_test(void) > > > { > > > char *kmem; > > > char __user *usermem; > > > - size_t size = 128 - KASAN_GRANULE_SIZE; > > > + /* > > > + * This is marked volatile to avoid __alloc_size() > > > + * noticing the intentionally out-of-bounds copys > > > + * being done on the allocation. > > > + */ > > > + volatile size_t size = 128 - KASAN_GRANULE_SIZE; > > > > Maybe OPTIMIZER_HIDE_VAR()? The normal version of that abuses an empty > > asm statement to hide the value from the compiler. > > Oh! I hadn't seen that before. Is that better than volatile in this > case? It forces the compiler to assume an arbitrary modification of the value, but doesn't force allocation of a stack slot like "volatile" does, so it probably generates nicer code? Not that it really matters here... It also has the difference that you can explicitly specify where the compiler's analysis should cut off, instead of just doing it on every access to a variable. But I guess maybe in this case, that's an argument in favor of "volatile"? I don't know... I guess maybe "volatile" does make sense here.