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 BC061C4828F for ; Fri, 9 Feb 2024 23:14:40 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 595E56B0075; Fri, 9 Feb 2024 18:14:40 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 5464F6B0078; Fri, 9 Feb 2024 18:14:40 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3C0526B007D; Fri, 9 Feb 2024 18:14:40 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 2BAAD6B0075 for ; Fri, 9 Feb 2024 18:14:40 -0500 (EST) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id EEBE7804F4 for ; Fri, 9 Feb 2024 23:14:39 +0000 (UTC) X-FDA: 81773821878.02.6226540 Received: from mail-oi1-f171.google.com (mail-oi1-f171.google.com [209.85.167.171]) by imf13.hostedemail.com (Postfix) with ESMTP id 113002001F for ; Fri, 9 Feb 2024 23:14:37 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf13.hostedemail.com: domain of dcvernet@gmail.com designates 209.85.167.171 as permitted sender) smtp.mailfrom=dcvernet@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1707520478; 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; bh=NRPj+jysUNcaTqxrb9blVKYV08fOWVEOBJPFpCCyyAQ=; b=Ro1SGVLS+a4c7ALSI7Hmls92Vyv5hMl9v3rHgoerV2Pxn8G7OWdm0yAUz8QEKXnice8RMF TePYGenc1ACQezA2AfBgeUyl7Ye/Oqm77UkTyH5niYcA6rOnVpAxGhd9bK5AaqO5WOXgFJ l8kLPkFUq4fi6XtPKJJvQPXFtdfIDCQ= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf13.hostedemail.com: domain of dcvernet@gmail.com designates 209.85.167.171 as permitted sender) smtp.mailfrom=dcvernet@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1707520478; a=rsa-sha256; cv=none; b=KGGnMpUVWkdfN2gF8geL+QFH0GML2UbrOn5VtOunQRaZBFDzhtJIigS5KtW6rPoYFOP8bi 6mJglS2zkCdpkFiYdhyIgDOrxUJM5mIsJyARBSFmo3xqQ5tqtSgVNt0yNoqMarv72CZKBM l0szOjKsiE/l3QwBVNDyEVZqJt1WNJI= Received: by mail-oi1-f171.google.com with SMTP id 5614622812f47-3bbc649c275so869846b6e.0 for ; Fri, 09 Feb 2024 15:14:37 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707520477; x=1708125277; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=NRPj+jysUNcaTqxrb9blVKYV08fOWVEOBJPFpCCyyAQ=; b=MASOvXgztetiPTlBkqQAlA/CgASkuuuFFX7ejEGQX1Jps1x8ItFGmcva73qetzVipK hkM3PlkLPYgPJURKSyDECFYON1wNByen0xvyfjBVd/TmIAUa3+3U+t+mzfQuvvzyGfZR KJgxb3BmQ/lwZ2a2zwOoZgb1qpggQlyT52Jocg82seOcjKcLERxvheWet/ACftOR/XRf GDOatQRzpIWqOp+SpigYPZIEdQaSJr9y340OTBquMmOtnP68F7sFhKHXPV/TnAirvKYx tsta+f46Lc7+9bh8W7SvWB4+XmWb0XMb4CUUwdiAKEEBebcF+VRWrwRWiz1pJ+2W5Cv5 hnWA== X-Forwarded-Encrypted: i=1; AJvYcCUBwYDhbqA+OgoKk1MvSLmDKc1qNEdrhX6nEjpFV/tn7Aaz3w+aOoJDaZWj8LhCXNCkWOAqg3RMh1tqPOIHjqV5ZbM= X-Gm-Message-State: AOJu0Yw6IdK191qllTpjNR4EKmddaG5f7xJTvyQuaKb3fKwkmid5saaL YwOP9ctdW6p2EbkW/JwqJ2GNnkFfn0DzpbQfKqL4aQdCHwSEA50LEt0mgeviYdUvPQ== X-Google-Smtp-Source: AGHT+IGdE53EhunSk/Cv0mEfpIYVJZ1Kx2cabFyRtVSFQtVa7WI2F3wVrXIBAzYPRAHW6QZL9KTjYA== X-Received: by 2002:a05:6808:120d:b0:3be:7321:a572 with SMTP id a13-20020a056808120d00b003be7321a572mr605967oil.17.1707520477059; Fri, 09 Feb 2024 15:14:37 -0800 (PST) X-Forwarded-Encrypted: i=1; AJvYcCXnR6nkcTCpefaah5HaxdXcY3wQGZhioualvKVIK90tIuAyUb3YpheIBr55/7AUSMMT4p9XcLWgSWyoOnJHlGxUMHhCTBVtZPFnFmYIE5QLJ3wuSS0C2oh9EEoQ1mww06UL+lUhuJv2stBzQ8mqrxsrN4UFzWVH8TNkeeLSIif9AEhsiE5NK8uGyFs4wJniZPfGkgfz2noJzfBFLsaT75ZSsrp/wUW7cNt++jJV6c8DYuyTG9Vxvu+Q89q42vubLc/4QfIkluHsStcACtLuMebPB8CgWeyTA7dAIKyMaSAX6YIuAwXZDKaUqmI/anhGpBlqnz6Vml0MKjtoH5BMWzGzhkWmuzCwR6GP6o4nug5GKM0cXyWx8/BYsCzBKdtn7qghOrPE+VkObeY39kE= Received: from maniforge.lan (c-24-1-27-177.hsd1.il.comcast.net. [24.1.27.177]) by smtp.gmail.com with ESMTPSA id ay20-20020a05620a179400b007855f767745sm163291qkb.73.2024.02.09.15.14.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 09 Feb 2024 15:14:36 -0800 (PST) Date: Fri, 9 Feb 2024 17:14:33 -0600 From: David Vernet To: Alexei Starovoitov Cc: bpf@vger.kernel.org, daniel@iogearbox.net, andrii@kernel.org, memxor@gmail.com, eddyz87@gmail.com, tj@kernel.org, brho@google.com, hannes@cmpxchg.org, lstoakes@gmail.com, akpm@linux-foundation.org, urezki@gmail.com, hch@infradead.org, linux-mm@kvack.org, kernel-team@fb.com Subject: Re: [PATCH v2 bpf-next 17/20] selftests/bpf: Add unit tests for bpf_arena_alloc/free_pages Message-ID: <20240209231433.GE975217@maniforge.lan> References: <20240209040608.98927-1-alexei.starovoitov@gmail.com> <20240209040608.98927-18-alexei.starovoitov@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="2B/LGZNqLS+Y8GQz" Content-Disposition: inline In-Reply-To: <20240209040608.98927-18-alexei.starovoitov@gmail.com> User-Agent: Mutt/2.2.12 (2023-09-09) X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 113002001F X-Stat-Signature: tasbx4i788d9ery989cjunn7z3o4656g X-HE-Tag: 1707520477-520789 X-HE-Meta: U2FsdGVkX1/Ge7D+SgMk72joJhncDFAAOPk/XTPZt14G5SofdadX3NQbKsRDpAFkvVf8kGBQallCbM2SqnxFJpdIm0wvz8Ku7bwKUC9Z34Nw9qr7BLLtHKqzUPcXWoPmwT1NpcPjyAfepDmOdFRcdcLInNIhPUkGqEeIHBUU7D05N9a9mHIXTgTFBouuMh8d97oz/9IiJyuqrfkHUKG39/DQNtq9yGAeK3ZqJ3X7VVQqyRMpgjlZZEzDGQemk0VyA+yCi79EIQPkhgHJhLj+LkzGOvZWEN0x4KlyaSXe2h8GxWD73A7UKYR37VYdQ8lMzh6ev0gfNgtsi+E4E/iAHx10QQGsEi/sN3c4QLDm/45Mup3nk1wnCN/jEhRP3II+DgryRRe4g27YsFGxtzuWPZXJE4SEdk1C3v/Ka/YgwHNUDQRB68+cLVFs9n7bNkddZTqiwILVTx6KmUB8kQ29NhjB+Ha2o4NOtxqOh+rerAf4/6/+3I8tpoq+/5vTozZtwiJa1hgTGuZsy3eqboUpKlQxS7oaQKOw3fxYqCh1T7VrXR1PYwjmtTUlofF7NIPjiU1END8rVVaYoKKxxZ32mZUbwhFi1lBIqI6RRCQnEJWBvLlYEfot9jSieZwnFoc8prN/+R2CaSq4cjZmZRdeVrIJ20ZQe8GNkcHGQ4tNLfbXQYPSthOBVUA129GjerOxpfBok0vKassBlL3Q9fcZ9rLooPWbRQhXdi/vcsyGxTKnxzfgdV9gd8FhcEQdG8SjNEAQIMmVQTx4BbYTP+S6/JFJHHCeUW2h7hGD6rky9JfisLNFbc40PAqqAGshKvmGSY7R06PD347Wfbv3DnD/XObiSpY1XND4ehftH7J24YmvjM/8wwAeS61WYuiGYOOAqMLNNe/U4QYK9DzmYTuQgls+0dmq7WZSudkuk7kEGRV/7bzE+Aasn6TAe0lRE2jNSS4ALxpKH0LnRsu9rC2 V4Et6BRG N6OIrWhQPrxHJol36O8XWvvs6LwTWMJxuNw7q2Olc4nUoO18ZxrZMhVyfAJ7Wym0A4y5tjt3yBygI+oh9gZiXxdzFZO7h5bO0TSS70NqhTp0Y/nOB12LijXtrDn5SpUx/8Ufo78Yaqr7d/dt8j88wLh+P2O3qasdrtCvUibjM/vfA6GNBS20sr6ANz/MW8CgJ/3jhQ8kr3Oe+XvIJkr5Cvtf2Q9hZeYM60qwHMO7/qk6bZUL3yK+eotZSEzzrq5S7szkoaFcSCi3U92ThePs4D5mTB36++tmn7LyXsGYugTZVUTtSQ/AThswXQfMcHIMgrWEBRvZtMbzDnSyXTQo9j0Xnxh4aLnPn02HZ8Es1q8HTy1C0KJ/tsqUjKebvl5Pp9PXRXLdp/E9ncIiwrJfvLZHMwj5l0bz7gaPx4K11ThZKa0wnwY2N9BDjHBv1QcT/KWrS3jdSg6p0yibpzx+mZX/eoOP/zDymt0UqPXZH/QhFVwxEFpH0Pf5/fcVuItmQfIWs19V7o7xhaf4G2/g9JaKlRODKbrHxXemTosoSRMsCK7CZtnBfkuewtvmVVovxXIzqnrAx5FmgyWmBUB9PzG9cj5lTMmHo8h06l0f6h7sd/2ScJQ6peH8blDaGFjGIchpd 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: List-Subscribe: List-Unsubscribe: --2B/LGZNqLS+Y8GQz Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Feb 08, 2024 at 08:06:05PM -0800, Alexei Starovoitov wrote: > From: Alexei Starovoitov >=20 > Add unit tests for bpf_arena_alloc/free_pages() functionality > and bpf_arena_common.h with a set of common helpers and macros that > is used in this test and the following patches. >=20 > Also modify test_loader that didn't support running bpf_prog_type_syscall > programs. >=20 > Signed-off-by: Alexei Starovoitov > --- > tools/testing/selftests/bpf/DENYLIST.aarch64 | 1 + > tools/testing/selftests/bpf/DENYLIST.s390x | 1 + > .../testing/selftests/bpf/bpf_arena_common.h | 70 ++++++++++++++ > .../selftests/bpf/prog_tests/verifier.c | 2 + > .../selftests/bpf/progs/verifier_arena.c | 91 +++++++++++++++++++ > tools/testing/selftests/bpf/test_loader.c | 9 +- > 6 files changed, 172 insertions(+), 2 deletions(-) > create mode 100644 tools/testing/selftests/bpf/bpf_arena_common.h > create mode 100644 tools/testing/selftests/bpf/progs/verifier_arena.c >=20 > diff --git a/tools/testing/selftests/bpf/DENYLIST.aarch64 b/tools/testing= /selftests/bpf/DENYLIST.aarch64 > index 5c2cc7e8c5d0..8e70af386e52 100644 > --- a/tools/testing/selftests/bpf/DENYLIST.aarch64 > +++ b/tools/testing/selftests/bpf/DENYLIST.aarch64 > @@ -11,3 +11,4 @@ fill_link_info/kprobe_multi_link_info # bpf_= program__attach_kprobe_mu > fill_link_info/kretprobe_multi_link_info # bpf_program__attach_k= probe_multi_opts unexpected error: -95 > fill_link_info/kprobe_multi_invalid_ubuff # bpf_program__attach_k= probe_multi_opts unexpected error: -95 > missed/kprobe_recursion # missed_kprobe_recursi= on__attach unexpected error: -95 (errno 95) > +verifier_arena # JIT does not support = arena > diff --git a/tools/testing/selftests/bpf/DENYLIST.s390x b/tools/testing/s= elftests/bpf/DENYLIST.s390x > index 1a63996c0304..ded440277f6e 100644 > --- a/tools/testing/selftests/bpf/DENYLIST.s390x > +++ b/tools/testing/selftests/bpf/DENYLIST.s390x > @@ -3,3 +3,4 @@ > exceptions # JIT does not support calling kfunc bpf_throw = (exceptions) > get_stack_raw_tp # user_stack corrupted user sta= ck (no backchain userspace) > stacktrace_build_id # compare_map_keys stackid_hmap= vs. stackmap err -2 errno 2 (?) > +verifier_arena # JIT does not support arena > diff --git a/tools/testing/selftests/bpf/bpf_arena_common.h b/tools/testi= ng/selftests/bpf/bpf_arena_common.h > new file mode 100644 > index 000000000000..07849d502f40 > --- /dev/null > +++ b/tools/testing/selftests/bpf/bpf_arena_common.h > @@ -0,0 +1,70 @@ > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ > +/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */ > +#pragma once > + > +#ifndef WRITE_ONCE > +#define WRITE_ONCE(x, val) ((*(volatile typeof(x) *) &(x)) =3D (val)) > +#endif > + > +#ifndef NUMA_NO_NODE > +#define NUMA_NO_NODE (-1) > +#endif > + > +#ifndef arena_container_of Why is this ifndef required if we have a pragma once above? > +#define arena_container_of(ptr, type, member) \ > + ({ \ > + void __arena *__mptr =3D (void __arena *)(ptr); \ > + ((type *)(__mptr - offsetof(type, member))); \ > + }) > +#endif > + > +#ifdef __BPF__ /* when compiled as bpf program */ > + > +#ifndef PAGE_SIZE > +#define PAGE_SIZE __PAGE_SIZE > +/* > + * for older kernels try sizeof(struct genradix_node) > + * or flexible: > + * static inline long __bpf_page_size(void) { > + * return bpf_core_enum_value(enum page_size_enum___l, __PAGE_SIZE___l= ) ?: sizeof(struct genradix_node); > + * } > + * but generated code is not great. > + */ > +#endif > + > +#if defined(__BPF_FEATURE_ARENA_CAST) && !defined(BPF_ARENA_FORCE_ASM) > +#define __arena __attribute__((address_space(1))) > +#define cast_kern(ptr) /* nop for bpf prog. emitted by LLVM */ > +#define cast_user(ptr) /* nop for bpf prog. emitted by LLVM */ > +#else > +#define __arena > +#define cast_kern(ptr) bpf_arena_cast(ptr, BPF_ARENA_CAST_KERN, 1) > +#define cast_user(ptr) bpf_arena_cast(ptr, BPF_ARENA_CAST_USER, 1) > +#endif > + > +void __arena* bpf_arena_alloc_pages(void *map, void __arena *addr, __u32= page_cnt, > + int node_id, __u64 flags) __ksym __weak; > +void bpf_arena_free_pages(void *map, void __arena *ptr, __u32 page_cnt) = __ksym __weak; > + > +#else /* when compiled as user space code */ > + > +#define __arena > +#define __arg_arena > +#define cast_kern(ptr) /* nop for user space */ > +#define cast_user(ptr) /* nop for user space */ > +__weak char arena[1]; > + > +#ifndef offsetof > +#define offsetof(type, member) ((unsigned long)&((type *)0)->member) > +#endif > + > +static inline void __arena* bpf_arena_alloc_pages(void *map, void *addr,= __u32 page_cnt, > + int node_id, __u64 flags) > +{ > + return NULL; > +} > +static inline void bpf_arena_free_pages(void *map, void __arena *ptr, __= u32 page_cnt) > +{ > +} > + > +#endif > diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/te= sting/selftests/bpf/prog_tests/verifier.c > index 9c6072a19745..985273832f89 100644 > --- a/tools/testing/selftests/bpf/prog_tests/verifier.c > +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c > @@ -4,6 +4,7 @@ > =20 > #include "cap_helpers.h" > #include "verifier_and.skel.h" > +#include "verifier_arena.skel.h" > #include "verifier_array_access.skel.h" > #include "verifier_basic_stack.skel.h" > #include "verifier_bitfield_write.skel.h" > @@ -118,6 +119,7 @@ static void run_tests_aux(const char *skel_name, > #define RUN(skel) run_tests_aux(#skel, skel##__elf_bytes, NULL) > =20 > void test_verifier_and(void) { RUN(verifier_and); } > +void test_verifier_arena(void) { RUN(verifier_arena); } > void test_verifier_basic_stack(void) { RUN(verifier_basic_stack= ); } > void test_verifier_bitfield_write(void) { RUN(verifier_bitfield_wr= ite); } > void test_verifier_bounds(void) { RUN(verifier_bounds); } > diff --git a/tools/testing/selftests/bpf/progs/verifier_arena.c b/tools/t= esting/selftests/bpf/progs/verifier_arena.c > new file mode 100644 > index 000000000000..0e667132ef92 > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/verifier_arena.c > @@ -0,0 +1,91 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */ > + > +#include > +#include > +#include > +#include "bpf_misc.h" > +#include "bpf_experimental.h" > +#include "bpf_arena_common.h" > + > +struct { > + __uint(type, BPF_MAP_TYPE_ARENA); > + __uint(map_flags, BPF_F_MMAPABLE); > + __uint(max_entries, 2); /* arena of two pages close to 32-bit boundary*/ > + __ulong(map_extra, (1ull << 44) | (~0u - __PAGE_SIZE * 2 + 1)); /* star= t of mmap() region */ > +} arena SEC(".maps"); > + > +SEC("syscall") > +__success __retval(0) > +int basic_alloc1(void *ctx) > +{ > + volatile int __arena *page1, *page2, *no_page, *page3; > + > + page1 =3D bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0); > + if (!page1) > + return 1; > + *page1 =3D 1; > + page2 =3D bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0); > + if (!page2) > + return 2; > + *page2 =3D 2; > + no_page =3D bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0); > + if (no_page) > + return 3; > + if (*page1 !=3D 1) > + return 4; > + if (*page2 !=3D 2) > + return 5; > + bpf_arena_free_pages(&arena, (void __arena *)page2, 1); > + if (*page1 !=3D 1) > + return 6; > + if (*page2 !=3D 0) /* use-after-free should return 0 */ So I know that longer term the plan is to leverage exceptions and have us exit and unwind the program here, but I think it's somewhat important to underscore how significant of a usability improvement that will be. Reading 0 isn't terribly uncommon for typical scheduling use cases. For example, if we stored a set of cpumasks in arena pages, we may AND them together and not be concerned if there are no CPUs set as that would be a perfectly normal state. E.g. if we're using arena cpumasks to track idle cores and a task's allowed CPUs, and we AND them together and see 0. We'd just assume there were no idle cores available on the system. Another example would be scx_nest where we would incorrectly think that a nest didn't have enough cores, seeing if a task could run in a domain, etc. Obviously it's way better for us to actually have arenas in the interim so this is fine for now, but UAF bugs could potentially be pretty painful until we get proper exception unwinding support. Otherwise, in terms of usability, this looks really good. The only thing to bear in mind is that I don't think we can fully get away from kptrs that will have some duplicated logic compared to what we can enable in an arena. For example, we will have to retain at least some of the struct cpumask * kptrs for e.g. copying a struct task_struct's struct cpumask *cpus_ptr field. For now, we could iterate over the cpumask and manually set the bits, so maybe even just supporting bpf_cpumask_test_cpu() would be enough (though donig a bitmap_copy() would be better of course)? This is probably fine for most use cases as we'd likely only be doing struct cpumask * -> arena copies on slowpaths. But is there any kind of more generalized integration we want to have between arenas and kptrs? Not sure, can't think of any off the top of my head. > + return 7; > + page3 =3D bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0); > + if (!page3) > + return 8; > + *page3 =3D 3; > + if (page2 !=3D page3) > + return 9; > + if (*page1 !=3D 1) > + return 10; Should we also test doing a store after an arena has been freed? > + return 0; > +} --2B/LGZNqLS+Y8GQz Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQRBxU1So5MTLwphjdFZ5LhpZcTzZAUCZcax2QAKCRBZ5LhpZcTz ZJG4AQCkFmA10tQnMrnqflB6zJBOlBX2iv7ItVv/DeUYUUP9mQD/Qdq98ieEmfwz GeHNlL4FND0GdDv7KsduwHLwznGtRAY= =NPrb -----END PGP SIGNATURE----- --2B/LGZNqLS+Y8GQz--