Message ID | 20221029025433.2533810-2-keescook@chromium.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp1153781wru; Fri, 28 Oct 2022 19:58:11 -0700 (PDT) X-Google-Smtp-Source: AMsMyM7OcAEA/jB2vIeJbksH6RGylx9jJvqriW8NPlUDg9wZJ3W4NK1k4icyblwH8mQ2qt15Fz4K X-Received: by 2002:a17:902:ef91:b0:186:f931:db98 with SMTP id iz17-20020a170902ef9100b00186f931db98mr2271955plb.166.1667012291193; Fri, 28 Oct 2022 19:58:11 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1667012291; cv=none; d=google.com; s=arc-20160816; b=OSWx3ksLTgk+Y3MTIW2n8qRZuxyV2iB94fONZVJuXSkYhEZGicT6Dqwg0v/KtvXEJv sIQP8kOAr35ZOA5G5ENrdqjrFnomgGy3CMagFBed8jmvQYSZyFKPRgIsiz1mJUdmzKyd hFv5GGexZSn1UGsTDcN2qdkpVS2exz1ueLtaCq2wuSBf6SxySVNPbmQzOSQ0uCFjIDlK SpyAfN6YncDV29FhaqZyYamablGNLXT1IzcQKFuYHRoVagxY1eQoUH4l6OmFnh2WUvpR cZ4KQTVdjYFdxPGQwflTnKwBTLMo+3AY0sQZ75GqMlyIktGZvWt9EMdZYyQKkcbta30c hYnA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=8tl37lwqGAPyFTOLYOb65cx9juvzNTvoWHjwD7xYuFQ=; b=hAD2k+ahh6Wbe43WTEPDx/xfgmofJu/mzoBX7Wa1B4Emhcd4s5UgAdD2qml9ms3whn HMzpNMzjz+KE36XeB1jNJqSGphjFSD7AqbNk0Zm1wZprhaWu9Y9XCO0FghOFn+2iEhfH 0eGqpX5Fvc61I4aIwm1jp/FkNbUEAvAym5MSikTblPCu4kBPUe0wkr28AcF21VW26ehl klfaXVNeHtUDnrTuOiiwMfTLurQRf33BRCaT3bIa7eCvv3ra92P9VSOl28FYbq3owA7L 56KT6cKLRUrXmEOWBCdhb5MctdUBETrhw+oAAbbyS3L8jf9gM74Q8p9qeokl4/DmJcuy 0aAA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=Cd9tmgsJ; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id y199-20020a6264d0000000b0056c07df95d8si496932pfb.210.2022.10.28.19.57.58; Fri, 28 Oct 2022 19:58:11 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=Cd9tmgsJ; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229816AbiJ2CzU (ORCPT <rfc822;pusanteemu@gmail.com> + 99 others); Fri, 28 Oct 2022 22:55:20 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40718 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229817AbiJ2CzQ (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 28 Oct 2022 22:55:16 -0400 Received: from mail-pj1-x102c.google.com (mail-pj1-x102c.google.com [IPv6:2607:f8b0:4864:20::102c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 590812494B for <linux-kernel@vger.kernel.org>; Fri, 28 Oct 2022 19:54:38 -0700 (PDT) Received: by mail-pj1-x102c.google.com with SMTP id v4-20020a17090a088400b00212cb0ed97eso6045328pjc.5 for <linux-kernel@vger.kernel.org>; Fri, 28 Oct 2022 19:54:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=8tl37lwqGAPyFTOLYOb65cx9juvzNTvoWHjwD7xYuFQ=; b=Cd9tmgsJJdWf0loxgAfE2HPeppCYbVz4D5gJ5tQcKB6L+ovA12Ew9mChsqPQXbuSUK P+EKoy+7WwL8q2rj931dqINtyZh6kEENAED1WS3uD8thf4KfTiTnMhSt3//hP3fVmfyz 3nim0DVM7yPvIiHCyNEkq1nGp3MR8CSWdqGbo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=8tl37lwqGAPyFTOLYOb65cx9juvzNTvoWHjwD7xYuFQ=; b=xDv2/7YxGUmV5JbvKfc42+m/6ZGdWzkv4wKDfoZXialVI0rJNW3mPVuUOOvIqD8XU3 Dfk8fyjbCiEV7wxwAqJBMHw5mVcf4OiR6nnpx8FuEKnNBHkPY/454TgPg+vJTi7GdlI3 qgPu9U8WfMnI0qUg/W1MdX6ytUoq7eg5985bv3qdlELKb0R6H5qAkb3fPZQUgCLoJNHw chqba8QKGKz0J2s73yRvCOSrz81c6KKHvmdMUSVPqgz448rh6ZlLad6mGFuIiEtHBsVT 28fAWetd7oFpHXHwKuCiSvLvNBYsp32aw9gqxUk1Mn4u0Nk2vIg4xG1kJrEZkLY3IMOM jA4g== X-Gm-Message-State: ACrzQf1oLgd6ay4twia00/07Az7wXX/xgC8eanPfR33o1Ml8gQiHfS0f uobg4SGZU3UgzZJjYJZ5OArpRg== X-Received: by 2002:a17:902:6b45:b0:186:878e:3b92 with SMTP id g5-20020a1709026b4500b00186878e3b92mr2222756plt.173.1667012077781; Fri, 28 Oct 2022 19:54:37 -0700 (PDT) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id p68-20020a625b47000000b0056b9c2699cesm170223pfb.46.2022.10.28.19.54.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 28 Oct 2022 19:54:34 -0700 (PDT) From: Kees Cook <keescook@chromium.org> To: Alexei Starovoitov <ast@kernel.org> Cc: Kees Cook <keescook@chromium.org>, Daniel Borkmann <daniel@iogearbox.net>, John Fastabend <john.fastabend@gmail.com>, Andrii Nakryiko <andrii@kernel.org>, Martin KaFai Lau <martin.lau@linux.dev>, Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>, KP Singh <kpsingh@kernel.org>, Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>, Jiri Olsa <jolsa@kernel.org>, bpf@vger.kernel.org, linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org Subject: [PATCH bpf-next v2 2/3] bpf/verifier: Use kmalloc_size_roundup() to match ksize() usage Date: Fri, 28 Oct 2022 19:54:31 -0700 Message-Id: <20221029025433.2533810-2-keescook@chromium.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221029024444.gonna.633-kees@kernel.org> References: <20221029024444.gonna.633-kees@kernel.org> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=2473; h=from:subject; bh=cAZzQf4nJI/2AavYPElJQqPl11xZNu41P29+G33SJ9M=; b=owEBbQKS/ZANAwAKAYly9N/cbcAmAcsmYgBjXJXnQGC8fPZwalh9l14/jG4kobQw8yjDcH8NbjSJ /XPIE3yJAjMEAAEKAB0WIQSlw/aPIp3WD3I+bhOJcvTf3G3AJgUCY1yV5wAKCRCJcvTf3G3AJtu9EA CXJjb2SNBE0dlmaXWQ51VUbBpWbuJYO6erazxkwDl20aLVHHY3Axth5HAjZff0RAgI1VuEgK0XZp1r RE0TT1eYl9G5ixdAC59qd1CyEHIcrq8+ifEZo8FgjuCM+csjbDGyrKYcgszia+gdt0jTBIbTc9RPDk h49CV0gy5UmirPGgX8dGIyGnpKI4FNyJSxPrtKEYB1NNT0NKDCYYx+RfELfzFCEUHszzQjq2QGPcoS MQCDLvOy6b2RiQLtsraXNMaGqb45mx9P5O9CTysRsnzDMvCA7NXwCuQuXsdjO/2HsBfxhz7Ar/YR9W Q9PmyZ/L0otSPcROBpRG2q6CUmDpOthzfBzcW58TTyDeLip32DmQpFbyiXtSgKMujj/DUh8fnY/DR8 2j1VH+mqdaYP7FiWIMA+ed88554A/22x76JzNS5Caj75Gk5klIWA9DRXxqyAIhkWj0AZZY0oDA+/bI BaXI9irmdN/ojaCMS7GiewpfSz1XqXzoo+a2/IHb6qtOmcwGzuRXjvvCvZc3nO7FmlQUzB23V63ndC YIDPWXj/8EpTHHzYIkslH/ROZny/WlLvaMSrDlNnM8CWhQz5RGWiBaLDqtr2+lr8CzFydwOd9JaxoT 275n7Yb5433TpvdeSuKo/IQkPJI9tpzIZx8/08+btiQkdvPTHYsFe9hnV9Iw== X-Developer-Key: i=keescook@chromium.org; a=openpgp; fpr=A5C3F68F229DD60F723E6E138972F4DFDC6DC026 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1747988995757161999?= X-GMAIL-MSGID: =?utf-8?q?1747989080342626694?= |
Series |
bpf/verifier: Use kmalloc_size_roundup() to match ksize() usage
|
|
Commit Message
Kees Cook
Oct. 29, 2022, 2:54 a.m. UTC
Round up allocations with kmalloc_size_roundup() so that the verifier's
use of ksize() is always accurate and no special handling of the memory
is needed by KASAN, UBSAN_BOUNDS, nor FORTIFY_SOURCE. Pass the new size
information back up to callers so they can use the space immediately,
so array resizing to happen less frequently as well.
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Martin KaFai Lau <martin.lau@linux.dev>
Cc: Song Liu <song@kernel.org>
Cc: Yonghong Song <yhs@fb.com>
Cc: KP Singh <kpsingh@kernel.org>
Cc: Stanislav Fomichev <sdf@google.com>
Cc: Hao Luo <haoluo@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: bpf@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
kernel/bpf/verifier.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
Comments
On 10/29/22 4:54 AM, Kees Cook wrote: > Round up allocations with kmalloc_size_roundup() so that the verifier's > use of ksize() is always accurate and no special handling of the memory > is needed by KASAN, UBSAN_BOUNDS, nor FORTIFY_SOURCE. Pass the new size > information back up to callers so they can use the space immediately, > so array resizing to happen less frequently as well. > [...] The commit message is a bit cryptic here without further context. Is this a bug fix or improvement? I read the latter, but it would be good to have more context here for reviewers (maybe Link tag pointing to some discussion or the like). Also, why is the kmalloc_size_roundup() not hidden for kmalloc callers, isn't this a tree-wide issue? Thanks, Daniel > kernel/bpf/verifier.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index eb8c34db74c7..1c040d27b8f6 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -1008,9 +1008,9 @@ static void *copy_array(void *dst, const void *src, size_t n, size_t size, gfp_t > if (unlikely(check_mul_overflow(n, size, &bytes))) > return NULL; > > - if (ksize(dst) < bytes) { > + if (ksize(dst) < ksize(src)) { > kfree(dst); > - dst = kmalloc_track_caller(bytes, flags); > + dst = kmalloc_track_caller(kmalloc_size_roundup(bytes), flags); > if (!dst) > return NULL; > } > @@ -1027,12 +1027,14 @@ static void *copy_array(void *dst, const void *src, size_t n, size_t size, gfp_t > */ > static void *realloc_array(void *arr, size_t old_n, size_t new_n, size_t size) > { > + size_t alloc_size; > void *new_arr; > > if (!new_n || old_n == new_n) > goto out; > > - new_arr = krealloc_array(arr, new_n, size, GFP_KERNEL); > + alloc_size = kmalloc_size_roundup(size_mul(new_n, size)); > + new_arr = krealloc(arr, alloc_size, GFP_KERNEL); > if (!new_arr) { > kfree(arr); > return NULL; > @@ -2504,9 +2506,11 @@ static int push_jmp_history(struct bpf_verifier_env *env, > { > u32 cnt = cur->jmp_history_cnt; > struct bpf_idx_pair *p; > + size_t alloc_size; > > cnt++; > - p = krealloc(cur->jmp_history, cnt * sizeof(*p), GFP_USER); > + alloc_size = kmalloc_size_roundup(size_mul(cnt, sizeof(*p))); > + p = krealloc(cur->jmp_history, alloc_size, GFP_USER); > if (!p) > return -ENOMEM; > p[cnt - 1].idx = env->insn_idx; >
On Tue, Nov 01, 2022 at 02:52:16PM +0100, Daniel Borkmann wrote: > On 10/29/22 4:54 AM, Kees Cook wrote: > > Round up allocations with kmalloc_size_roundup() so that the verifier's > > use of ksize() is always accurate and no special handling of the memory > > is needed by KASAN, UBSAN_BOUNDS, nor FORTIFY_SOURCE. Pass the new size > > information back up to callers so they can use the space immediately, > > so array resizing to happen less frequently as well. > > > [...] > > The commit message is a bit cryptic here without further context. Is this > a bug fix or improvement? I read the latter, but it would be good to have It's an improvement -- e.g. it depends on the recently added kmalloc_size_roundup() helper. > more context here for reviewers (maybe Link tag pointing to some discussion > or the like). Also, why is the kmalloc_size_roundup() not hidden for kmalloc > callers, isn't this a tree-wide issue? The main issue is that _most_ allocation callers want an explicitly sized allocation (and not "more"), and that dynamic runtime analysis tools (e.g. KASAN, UBSAN_BOUNDS, FORTIFY_SOURCE, etc) are looking for precise bounds checking (i.e. not something that is rounded up). A tiny handful of allocations were doing an implicit alloc/realloc loop that actually depended on ksize(), and didn't actually always call realloc. This has created a long series of bugs and problems over many years related to the runtime bounds checking, so these callers are finally being adjusted to _not_ depend on the ksize() side-effect, by doing one of several things: - tracking the allocation size precisely and just never calling ksize() at all[1]. - always calling realloc and not using ksize() at all. (This solution ends up actually be a subset of the next solution.) - using kmalloc_size_roundup() to explicitly round up the desired allocation size immediately[2]. The bpf/verifier case is this another of this latter case. Because some of the dynamic bounds checking depends on the size being an _argument_ to an allocator function (i.e. see the __alloc_size attribute), the ksize() users are rare, and it could waste local variables, it was been deemed better to explicitly separate the rounding up from the allocation itself[3]. Hopefully that helps clarify! :) -Kees [1] e.g.: https://git.kernel.org/linus/712f210a457d https://git.kernel.org/linus/72c08d9f4c72 [2] e.g.: https://git.kernel.org/netdev/net-next/c/12d6c1d3a2ad https://git.kernel.org/netdev/net-next/c/ab3f7828c979 https://git.kernel.org/netdev/net-next/c/d6dd508080a3 [3] https://lore.kernel.org/lkml/0ea1fc165a6c6117f982f4f135093e69cb884930.camel@redhat.com/
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index eb8c34db74c7..1c040d27b8f6 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -1008,9 +1008,9 @@ static void *copy_array(void *dst, const void *src, size_t n, size_t size, gfp_t if (unlikely(check_mul_overflow(n, size, &bytes))) return NULL; - if (ksize(dst) < bytes) { + if (ksize(dst) < ksize(src)) { kfree(dst); - dst = kmalloc_track_caller(bytes, flags); + dst = kmalloc_track_caller(kmalloc_size_roundup(bytes), flags); if (!dst) return NULL; } @@ -1027,12 +1027,14 @@ static void *copy_array(void *dst, const void *src, size_t n, size_t size, gfp_t */ static void *realloc_array(void *arr, size_t old_n, size_t new_n, size_t size) { + size_t alloc_size; void *new_arr; if (!new_n || old_n == new_n) goto out; - new_arr = krealloc_array(arr, new_n, size, GFP_KERNEL); + alloc_size = kmalloc_size_roundup(size_mul(new_n, size)); + new_arr = krealloc(arr, alloc_size, GFP_KERNEL); if (!new_arr) { kfree(arr); return NULL; @@ -2504,9 +2506,11 @@ static int push_jmp_history(struct bpf_verifier_env *env, { u32 cnt = cur->jmp_history_cnt; struct bpf_idx_pair *p; + size_t alloc_size; cnt++; - p = krealloc(cur->jmp_history, cnt * sizeof(*p), GFP_USER); + alloc_size = kmalloc_size_roundup(size_mul(cnt, sizeof(*p))); + p = krealloc(cur->jmp_history, alloc_size, GFP_USER); if (!p) return -ENOMEM; p[cnt - 1].idx = env->insn_idx;