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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2529CC07E9D for ; Sat, 24 Sep 2022 09:11:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9343F8E0003; Sat, 24 Sep 2022 05:11:33 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8E3278E0001; Sat, 24 Sep 2022 05:11:33 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7841C8E0003; Sat, 24 Sep 2022 05:11:33 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 65FC78E0001 for ; Sat, 24 Sep 2022 05:11:33 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 2D682A0F99 for ; Sat, 24 Sep 2022 09:11:33 +0000 (UTC) X-FDA: 79946410866.23.50EC640 Received: from mail-pj1-f45.google.com (mail-pj1-f45.google.com [209.85.216.45]) by imf07.hostedemail.com (Postfix) with ESMTP id AB45840004 for ; Sat, 24 Sep 2022 09:11:32 +0000 (UTC) Received: by mail-pj1-f45.google.com with SMTP id o99-20020a17090a0a6c00b002039c4fce53so8016408pjo.2 for ; Sat, 24 Sep 2022 02:11:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date; bh=i+DbZ92rhFWH/0UDN/zG8OtvV+E4X59I6BjaPOZbEQQ=; b=E5bOhboA/+6BWRDx1UWFFrMvOXvKoaBtGsWYlDmuj8X7rAkhnZpWRHTQ3LNDz7+mZj DgCRrEBR3MyfN1vHwttkAVt9C+sNXNl8rbk7MHBer0lal296QvNAuoGU6ltJiVjNpwJw ZbeC7n9zGD6mKw33TGHmgsxJslMQNugj0HBL4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date; bh=i+DbZ92rhFWH/0UDN/zG8OtvV+E4X59I6BjaPOZbEQQ=; b=G7/+8hrdqzTUs+j4BbrWLdKXRpFBDWkSMJcnkCuKlyC46E9i726z3RPO46LYQO7xr4 oVw1KRp+Z/DERszzXG/HOTlEgjDlahs4CDFtzgeSi9TxE2JOhknKYzyeJ1n1wHSSI+tj h4DdoDa54oN7sNrsLEqkMgNWGV+nuOBBOO0zP15u5MncDhbzKruoawG+zyz+sKQxZnmV xC4MhMIHp4/lQhI5uGswoiTDIIDngI7zTCuRC+SGnFSUHASDh+GS69CWFLwyD0wksgqJ ISTuBz299S6hrFfhxPHpQxTW5VaoDxMpsSn1wb6RGC3bbsqg7RCQLpEH7Tau5bzX37ac /DfA== X-Gm-Message-State: ACrzQf3U0Lm3fn+1PqA18bfQje5w/kAa7ItvwqBl+LvWCREsnPVlCMNC kfqZ70Hki/IfGH/hjxe2fnPwuw== X-Google-Smtp-Source: AMsMyM7r8bq+DxJ2Zg4nmPe+hShYURXJSZpppn8rw6aZz5OkkmasnCi/hTE7Baskd4vD2R6zimHrAA== X-Received: by 2002:a17:902:d4c9:b0:178:6d7b:c36f with SMTP id o9-20020a170902d4c900b001786d7bc36fmr12510856plg.19.1664010691458; Sat, 24 Sep 2022 02:11:31 -0700 (PDT) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id b13-20020aa7950d000000b005546fe4b127sm5590232pfp.78.2022.09.24.02.11.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 24 Sep 2022 02:11:30 -0700 (PDT) Date: Sat, 24 Sep 2022 02:11:29 -0700 From: Kees Cook To: Jakub Kicinski Cc: "David S. Miller" , Vlastimil Babka , Eric Dumazet , Paolo Abeni , netdev@vger.kernel.org, Greg Kroah-Hartman , Nick Desaulniers , David Rientjes , "Ruhl, Michael J" , Hyeonggon Yoo <42.hyeyoo@gmail.com>, Christoph Lameter , Pekka Enberg , Joonsoo Kim , Andrew Morton , Alex Elder , Josef Bacik , David Sterba , Sumit Semwal , Christian =?iso-8859-1?Q?K=F6nig?= , Jesse Brandeburg , Daniel Micay , Yonghong Song , Marco Elver , Miguel Ojeda , linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-btrfs@vger.kernel.org, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, linux-fsdevel@vger.kernel.org, intel-wired-lan@lists.osuosl.org, dev@openvswitch.org, x86@kernel.org, llvm@lists.linux.dev, linux-hardening@vger.kernel.org Subject: Re: [PATCH v2 03/16] skbuff: Proactively round up to kmalloc bucket size Message-ID: <202209240208.84F847F3B@keescook> References: <20220923202822.2667581-1-keescook@chromium.org> <20220923202822.2667581-4-keescook@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220923202822.2667581-4-keescook@chromium.org> ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1664010692; a=rsa-sha256; cv=none; b=THAAUPcsB/mOAd4l6AS14BHv8WEA6ruk4qa44186WyOYcng1RjC7RlIT1RYejTZyAaowDs ao+B9/7/68g/XFutsudTuW3pBxKnNAyG5Agn6TJac5Z1heIC84yUP6brxuihkPyLb4ieBN supoWfuwWUeRDJS4cX/lDzGNgtB/tLM= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=E5bOhboA; dmarc=pass (policy=none) header.from=chromium.org; spf=pass (imf07.hostedemail.com: domain of keescook@chromium.org designates 209.85.216.45 as permitted sender) smtp.mailfrom=keescook@chromium.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1664010692; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=i+DbZ92rhFWH/0UDN/zG8OtvV+E4X59I6BjaPOZbEQQ=; b=b6GMSMyx9veWwpMxi4zTGGhNPGy1zL0/LYJPGgxfc6zB09LyE9BZDgYiMRs0cPEngTcCdE R07QU+Rw0QHOXNKFHJBlg0hMVoLXyi/wdbtB3nO4hEPsZLYtjPEktmBT9oWkedd1sbD82t LB6+ZdbXTu7gX4u2954pnpjzc3ZR4OI= Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=E5bOhboA; dmarc=pass (policy=none) header.from=chromium.org; spf=pass (imf07.hostedemail.com: domain of keescook@chromium.org designates 209.85.216.45 as permitted sender) smtp.mailfrom=keescook@chromium.org X-Rspam-User: X-Stat-Signature: thxuceqqha3htyq6sut614tmng4hwis7 X-Rspamd-Queue-Id: AB45840004 X-Rspamd-Server: rspam06 X-HE-Tag: 1664010692-871553 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 23, 2022 at 01:28:09PM -0700, Kees Cook wrote: > Instead of discovering the kmalloc bucket size _after_ allocation, round > up proactively so the allocation is explicitly made for the full size, > allowing the compiler to correctly reason about the resulting size of > the buffer through the existing __alloc_size() hint. > > This will allow for kernels built with CONFIG_UBSAN_BOUNDS or the > coming dynamic bounds checking under CONFIG_FORTIFY_SOURCE to gain > back the __alloc_size() hints that were temporarily reverted in commit > 93dd04ab0b2b ("slab: remove __alloc_size attribute from __kmalloc_track_caller") > > Additionally tries to normalize size variables to u32 from int. Most > interfaces are using "int", but notably __alloc_skb uses unsigned int. > > Also fix some reverse Christmas tree and comments while touching nearby > code. Something in this patch is breaking things -- I've refactored it again to avoid overwriting the incoming size argument, and instead add a dedicated outgoing size variable. Here's what will be v3 ... --- net/core/skbuff.c | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 974bbbbe7138..9b5a9fb69d9d 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -346,11 +346,12 @@ EXPORT_SYMBOL(napi_build_skb); * memory is free */ static void *kmalloc_reserve(size_t size, gfp_t flags, int node, - bool *pfmemalloc) + bool *pfmemalloc, size_t *alloc_size) { void *obj; bool ret_pfmemalloc = false; + size = kmalloc_size_roundup(size); /* * Try a regular allocation, when that fails and we're not entitled * to the reserves, fail. @@ -369,6 +370,7 @@ static void *kmalloc_reserve(size_t size, gfp_t flags, int node, if (pfmemalloc) *pfmemalloc = ret_pfmemalloc; + *alloc_size = size; return obj; } @@ -400,7 +402,7 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask, { struct kmem_cache *cache; struct sk_buff *skb; - unsigned int osize; + size_t alloc_size; bool pfmemalloc; u8 *data; @@ -427,15 +429,15 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask, */ size = SKB_DATA_ALIGN(size); size += SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); - data = kmalloc_reserve(size, gfp_mask, node, &pfmemalloc); - if (unlikely(!data)) - goto nodata; - /* kmalloc(size) might give us more room than requested. + /* kmalloc(size) might give us more room than requested, so + * allocate the true bucket size up front. * Put skb_shared_info exactly at the end of allocated zone, * to allow max possible filling before reallocation. */ - osize = ksize(data); - size = SKB_WITH_OVERHEAD(osize); + data = kmalloc_reserve(size, gfp_mask, node, &pfmemalloc, &alloc_size); + if (unlikely(!data)) + goto nodata; + size = SKB_WITH_OVERHEAD(alloc_size); prefetchw(data + size); /* @@ -444,7 +446,7 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask, * the tail pointer in struct sk_buff! */ memset(skb, 0, offsetof(struct sk_buff, tail)); - __build_skb_around(skb, data, osize); + __build_skb_around(skb, data, alloc_size); skb->pfmemalloc = pfmemalloc; if (flags & SKB_ALLOC_FCLONE) { @@ -1709,6 +1711,7 @@ int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail, { int i, osize = skb_end_offset(skb); int size = osize + nhead + ntail; + size_t alloc_size; long off; u8 *data; @@ -1723,10 +1726,10 @@ int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail, if (skb_pfmemalloc(skb)) gfp_mask |= __GFP_MEMALLOC; data = kmalloc_reserve(size + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)), - gfp_mask, NUMA_NO_NODE, NULL); + gfp_mask, NUMA_NO_NODE, NULL, &alloc_size); if (!data) goto nodata; - size = SKB_WITH_OVERHEAD(ksize(data)); + size = SKB_WITH_OVERHEAD(alloc_size); /* Copy only real data... and, alas, header. This should be * optimized for the cases when header is void. @@ -6063,19 +6066,19 @@ static int pskb_carve_inside_header(struct sk_buff *skb, const u32 off, int i; int size = skb_end_offset(skb); int new_hlen = headlen - off; + size_t alloc_size; u8 *data; size = SKB_DATA_ALIGN(size); if (skb_pfmemalloc(skb)) gfp_mask |= __GFP_MEMALLOC; - data = kmalloc_reserve(size + - SKB_DATA_ALIGN(sizeof(struct skb_shared_info)), - gfp_mask, NUMA_NO_NODE, NULL); + data = kmalloc_reserve(size + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)), + gfp_mask, NUMA_NO_NODE, NULL, &alloc_size); if (!data) return -ENOMEM; - size = SKB_WITH_OVERHEAD(ksize(data)); + size = SKB_WITH_OVERHEAD(alloc_size); /* Copy real data, and all frags */ skb_copy_from_linear_data_offset(skb, off, data, new_hlen); @@ -6184,18 +6187,18 @@ static int pskb_carve_inside_nonlinear(struct sk_buff *skb, const u32 off, u8 *data; const int nfrags = skb_shinfo(skb)->nr_frags; struct skb_shared_info *shinfo; + size_t alloc_size; size = SKB_DATA_ALIGN(size); if (skb_pfmemalloc(skb)) gfp_mask |= __GFP_MEMALLOC; - data = kmalloc_reserve(size + - SKB_DATA_ALIGN(sizeof(struct skb_shared_info)), - gfp_mask, NUMA_NO_NODE, NULL); + data = kmalloc_reserve(size + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)), + gfp_mask, NUMA_NO_NODE, NULL, &alloc_size); if (!data) return -ENOMEM; - size = SKB_WITH_OVERHEAD(ksize(data)); + size = SKB_WITH_OVERHEAD(alloc_size); memcpy((struct skb_shared_info *)(data + size), skb_shinfo(skb), offsetof(struct skb_shared_info, frags[0])); -- 2.34.1 -- Kees Cook