linux-kernel - Re: [PATCH v2 2/2] bpf: add test for DEVMAP reuse

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cb4a9baeaa71e6512366267006907bf6608cca72.camel@gmail.com>
Date: Wed, 13 Aug 2025 14:54:01 -0700
From: Eduard Zingerman <eddyz87@...il.com>
To: Yureka Lilian <yuka@...a.dev>, Andrii Nakryiko <andrii@...nel.org>, 
 Alexei Starovoitov	 <ast@...nel.org>, Daniel Borkmann
 <daniel@...earbox.net>, Martin KaFai Lau	 <martin.lau@...ux.dev>, Song Liu
 <song@...nel.org>, Yonghong Song	 <yonghong.song@...ux.dev>, John Fastabend
 <john.fastabend@...il.com>, KP Singh	 <kpsingh@...nel.org>, Stanislav
 Fomichev <sdf@...ichev.me>, Hao Luo	 <haoluo@...gle.com>, Jiri Olsa
 <jolsa@...nel.org>
Cc: bpf@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] bpf: add test for DEVMAP reuse

On Wed, 2025-08-13 at 22:09 +0200, Yureka Lilian wrote:
> The test covers basic re-use of a pinned DEVMAP map,
> with both matching and mismatching parameters.
> 
> Signed-off-by: Yureka Lilian <yuka@...a.dev>
> ---

Thank you for adding the test case, please find a few comments below.

>  .../bpf/prog_tests/pinning_devmap_reuse.c     | 68 +++++++++++++++++++
>  .../selftests/bpf/progs/test_pinning_devmap.c | 20 ++++++
>  2 files changed, 88 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/pinning_devmap_reuse.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_pinning_devmap.c
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/pinning_devmap_reuse.c b/tools/testing/selftests/bpf/prog_tests/pinning_devmap_reuse.c
> new file mode 100644
> index 000000000..06befb03b
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/pinning_devmap_reuse.c
> @@ -0,0 +1,68 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +#include <test_progs.h>
> +
> +void test_pinning_devmap_reuse(void)
> +{
> +	const char *pinpath1 = "/sys/fs/bpf/pinmap1";
> +	const char *pinpath2 = "/sys/fs/bpf/pinmap2";
> +	const char *file = "./test_pinning_devmap.bpf.o";
> +	struct bpf_object *obj1 = NULL, *obj2 = NULL;
> +	int err;
> +	__u32 duration = 0;
> +	DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts);
> +
> +	/* load the object a first time */
> +	obj1 = bpf_object__open_file(file, NULL);

The test can be simplified by including test_pinning_devmap.skel.h and
calling test_pinning_devmap__open_and_load(), thus avoiding separate
calls to open_file and load.

> +	err = libbpf_get_error(obj1);
> +	if (CHECK(err, "first open", "err %d\n", err)) {
> +		obj1 = NULL;
> +		goto out;
> +	}
> +	err = bpf_object__load(obj1);
> +	if (CHECK(err, "first load", "err %d\n", err))

Please don't use CHECK in new tests.

> +		goto out;
> +
> +	/* load the object a second time, re-using the pinned map */
> +	obj2 = bpf_object__open_file(file, NULL);
> +	if (CHECK(err, "second open", "err %d\n", err)) {
> +		obj2 = NULL;
> +		goto out;
> +	}
> +	err = bpf_object__load(obj2);
> +	if (CHECK(err, "second load", "err %d\n", err))
> +		goto out;
> +
> +	/* we can close the reference safely without
> +	 * the map's refcount falling to 0
> +	 */
> +	bpf_object__close(obj1);
> +	obj1 = NULL;
> +
> +	/* now, swap the pins */
> +	err = renameat2(0, pinpath1, 0, pinpath2, RENAME_EXCHANGE);
> +	if (CHECK(err, "swap pins", "err %d\n", err))
> +		goto out;
> +
> +	/* load the object again, this time the re-use should fail */
> +	obj1 = bpf_object__open_file(file, NULL);
> +	err = libbpf_get_error(obj1);
> +	if (CHECK(err, "third open", "err %d\n", err)) {
> +		obj1 = NULL;
> +		goto out;
> +	}
> +	err = bpf_object__load(obj1);
> +	if (CHECK(err != -EINVAL, "param mismatch load", "err %d\n", err))
> +		goto out;
> +
> +out:
> +	unlink(pinpath1);
> +	unlink(pinpath2);
> +	if (obj1)
> +		bpf_object__close(obj1);

Nit: bpf_object__close() can handle NULLs.

> +	if (obj2)
> +		bpf_object__close(obj2);
> +}

[...]