From patchwork Thu Jun 1 00:08:17 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rhys Rustad-Elliott X-Patchwork-Id: 13262881 X-Patchwork-Delegate: bpf@iogearbox.net Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 283A6399 for ; Thu, 1 Jun 2023 00:08:36 +0000 (UTC) Received: from mail-4018.proton.ch (mail-4018.proton.ch [185.70.40.18]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B41FF124; Wed, 31 May 2023 17:08:31 -0700 (PDT) Date: Thu, 01 Jun 2023 00:08:17 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rhysre.net; s=protonmail2; t=1685578109; x=1685837309; bh=ZmyJWhx3fktYZQdFy32ON1t+Qe9jELp1qwHtWiiLc5o=; h=Date:From:Cc:Subject:Message-ID:Feedback-ID:From:To:Cc:Date: Subject:Reply-To:Feedback-ID:Message-ID:BIMI-Selector; b=YqKetOG3WbWUUv3XHh2wvRPvRb8Gty+KWMZ92uXnAXBtKzh3jY8TBgGg5BQUJ7J8c oPyHeFmv2QS3b1UQg62ZrWtNAfKA4P7YVvee0kYKlN0tvScB9E3TqXvRulDKCGYGig e4yBS/4o3qFRpMBrJDGinbdLB/ducPL2ii/NwKyE2rjn7R7N5QmOqaCAqKYQ0WPFC0 eW9exkcABk04r3r/b5QLorTeQtN5B9PeVRuMEBDKtg63WWYyy7qkFsitKupsxyCFQs beGXFRzFIiqTmTgLh96Mugy3NF17FEJUN6E6TT4gT81JlnL50favtern6UshI65JPb 4MWmjoV3TdJgA== From: Rhys Rustad-Elliott Cc: me@rhysre.net, Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Song Liu , Yonghong Song , John Fastabend , KP Singh , Stanislav Fomichev , Hao Luo , Jiri Olsa , Mykola Lysenko , Shuah Khan , linux-kernel@vger.kernel.org, bpf@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: [PATCH bpf] bpf: Fix elem_size not being set for inner maps Message-ID: <20230601000713.506358-1-me@rhysre.net> Feedback-ID: 51368404:user:proton Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Spam-Status: No, score=-0.7 required=5.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,MISSING_HEADERS,SPF_HELO_PASS,SPF_PASS, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net X-Patchwork-Delegate: bpf@iogearbox.net Commit d937bc3449fa ("bpf: make uniform use of array->elem_size everywhere in arraymap.c") changed array_map_gen_lookup to use array->elem_size instead of round_up(map->value_size, 8) as the element size when generating code to access a value in an array map. array->elem_size, however, is not set by bpf_map_meta_alloc when initializing an BPF_MAP_TYPE_ARRAY_OF_MAPS or BPF_MAP_TYPE_HASH_OF_MAPS. This results in array_map_gen_lookup incorrectly outputting code that always accesses index 0 in the array (as the index will be calculated via a multiplication with the element size, which is incorrectly set to 0). Set elem_size on the bpf_array object when allocating an array or hash of maps and add a selftest that accesses an inner map at a nonzero index to prevent regressions. Fixes: d937bc3449fa ("bpf: make uniform use of array->elem_size everywhere in arraymap.c") Signed-off-by: Rhys Rustad-Elliott --- kernel/bpf/map_in_map.c | 8 +++- .../map_in_map_inner_array_lookup.c | 33 ++++++++++++++ .../test_map_in_map_inner_array_lookup.c | 45 +++++++++++++++++++ 3 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/map_in_map_inner_array_lookup.c create mode 100644 tools/testing/selftests/bpf/progs/test_map_in_map_inner_array_lookup.c diff --git a/kernel/bpf/map_in_map.c b/kernel/bpf/map_in_map.c index 2c5c64c2a53b..8d65b12e0834 100644 --- a/kernel/bpf/map_in_map.c +++ b/kernel/bpf/map_in_map.c @@ -69,9 +69,13 @@ struct bpf_map *bpf_map_meta_alloc(int inner_map_ufd) /* Misc members not needed in bpf_map_meta_equal() check. */ inner_map_meta->ops = inner_map->ops; if (inner_map->ops == &array_map_ops) { + struct bpf_array *inner_array_meta = + container_of(inner_map_meta, struct bpf_array, map); + struct bpf_array *inner_array = container_of(inner_map, struct bpf_array, map); + + inner_array_meta->index_mask = inner_array->index_mask; + inner_array_meta->elem_size = round_up(inner_map->value_size, 8); inner_map_meta->bypass_spec_v1 = inner_map->bypass_spec_v1; - container_of(inner_map_meta, struct bpf_array, map)->index_mask = - container_of(inner_map, struct bpf_array, map)->index_mask; } fdput(f); diff --git a/tools/testing/selftests/bpf/prog_tests/map_in_map_inner_array_lookup.c b/tools/testing/selftests/bpf/prog_tests/map_in_map_inner_array_lookup.c new file mode 100644 index 000000000000..264d4788e5fd --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/map_in_map_inner_array_lookup.c @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: GPL-2.0-only + +#include + +#include "test_map_in_map_inner_array_lookup.skel.h" + +static int duration; + +void test_map_in_map_inner_array_lookup(void) +{ + int map1_fd, err; + int key = 3; + int val = 1; + struct test_map_in_map_inner_array_lookup *skel; + + skel = test_map_in_map_inner_array_lookup__open_and_load(); + if (CHECK(!skel, "skel_open", "failed to open&load skeleton\n")) + return; + + err = test_map_in_map_inner_array_lookup__attach(skel); + if (CHECK(err, "skel_attach", "skeleton attach failed: %d\n", err)) + goto cleanup; + + map1_fd = bpf_map__fd(skel->maps.inner_map1); + bpf_map_update_elem(map1_fd, &key, &val, 0); + usleep(1); + /* Probe should have set the element at index 3 to 2 */ + bpf_map_lookup_elem(map1_fd, &key, &val); + CHECK(val != 2, "inner1", "got %d != exp %d\n", val, 2); + +cleanup: + test_map_in_map_inner_array_lookup__destroy(skel); +} diff --git a/tools/testing/selftests/bpf/progs/test_map_in_map_inner_array_lookup.c b/tools/testing/selftests/bpf/progs/test_map_in_map_inner_array_lookup.c new file mode 100644 index 000000000000..c2c8f2fa451d --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_map_in_map_inner_array_lookup.c @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: GPL-2.0-only + +#include +#include + +struct inner_map { + __uint(type, BPF_MAP_TYPE_ARRAY); + __uint(max_entries, 5); + __type(key, int); + __type(value, int); +} inner_map1 SEC(".maps"); + +struct outer_map { + __uint(type, BPF_MAP_TYPE_HASH_OF_MAPS); + __uint(max_entries, 3); + __type(key, int); + __array(values, struct inner_map); +} outer_map1 SEC(".maps") = { + .values = { + [2] = &inner_map1, + }, +}; + +SEC("raw_tp/sys_enter") +int handle__sys_enter(void *ctx) +{ + int outer_key = 2, inner_key = 3; + int *val; + void *map; + + map = bpf_map_lookup_elem(&outer_map1, &outer_key); + if (!map) + return 1; + + val = bpf_map_lookup_elem(map, &inner_key); + if (!val) + return 1; + + if (*val == 1) + *val = 2; + + return 0; +} + +char _license[] SEC("license") = "GPL";