netdev - Re: [PATCH bpf v2 2/2] selftests/bpf: Add a test for using a cpumap from an freplace-to-XDP program

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Fri, 9 Dec 2022 10:07:15 -0800
From:   Yonghong Song <yhs@...a.com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>,
        Andrii Nakryiko <andrii@...nel.org>,
        Mykola Lysenko <mykolal@...com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Martin KaFai Lau <martin.lau@...ux.dev>,
        Song Liu <song@...nel.org>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        Stanislav Fomichev <sdf@...gle.com>,
        Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jesper Dangaard Brouer <hawk@...nel.org>
Cc:     Shuah Khan <shuah@...nel.org>, bpf@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH bpf v2 2/2] selftests/bpf: Add a test for using a cpumap
 from an freplace-to-XDP program



On 12/9/22 6:26 AM, Toke Høiland-Jørgensen wrote:
> This adds a simple test for inserting an XDP program into a cpumap that is
> "owned" by an XDP program that was loaded as PROG_TYPE_EXT (as libxdp
> does). Prior to the kernel fix this would fail because the map type
> ownership would be set to PROG_TYPE_EXT instead of being resolved to
> PROG_TYPE_XDP.
> 
> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>

LGTM with a small nit below.

Acked-by: Yonghong Song <yhs@...com>

> ---
>   .../selftests/bpf/prog_tests/fexit_bpf2bpf.c  | 53 +++++++++++++++++++
>   .../selftests/bpf/progs/freplace_progmap.c    | 24 +++++++++
>   tools/testing/selftests/bpf/testing_helpers.c | 24 ++++++++-
>   tools/testing/selftests/bpf/testing_helpers.h |  2 +
>   4 files changed, 101 insertions(+), 2 deletions(-)
>   create mode 100644 tools/testing/selftests/bpf/progs/freplace_progmap.c
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> index d1e32e792536..dac088217f0f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> @@ -500,6 +500,57 @@ static void test_fentry_to_cgroup_bpf(void)
>   	bind4_prog__destroy(skel);
>   }
>   
> +static void test_func_replace_progmap(void)
> +{
> +	struct bpf_cpumap_val value = { .qsize = 1 };
> +	struct bpf_object *obj, *tgt_obj = NULL;
> +	struct bpf_program *drop, *redirect;
> +	struct bpf_map *cpumap;
> +	int err, tgt_fd;
> +	__u32 key = 0;
> +
> +	err = bpf_prog_test_open("freplace_progmap.bpf.o", BPF_PROG_TYPE_UNSPEC, &obj);
> +	if (!ASSERT_OK(err, "prog_open"))
> +		return;
> +
> +	err = bpf_prog_test_load("xdp_dummy.bpf.o", BPF_PROG_TYPE_UNSPEC, &tgt_obj, &tgt_fd);
> +	if (!ASSERT_OK(err, "tgt_prog_load"))
> +		goto out;
> +
> +	drop = bpf_object__find_program_by_name(obj, "xdp_drop_prog");
> +	redirect = bpf_object__find_program_by_name(obj, "xdp_cpumap_prog");
> +	cpumap = bpf_object__find_map_by_name(obj, "cpu_map");
> +
> +	if (!ASSERT_OK_PTR(drop, "drop") || !ASSERT_OK_PTR(redirect, "redirect") ||
> +	    !ASSERT_OK_PTR(cpumap, "cpumap"))
> +		goto out;
> +
> +	/* Change the 'redirect' program type to be a PROG_TYPE_EXT
> +	 * with an XDP target
> +	 */
> +	bpf_program__set_type(redirect, BPF_PROG_TYPE_EXT);
> +	bpf_program__set_expected_attach_type(redirect, 0);
> +	err = bpf_program__set_attach_target(redirect, tgt_fd, "xdp_dummy_prog");
> +	if (!ASSERT_OK(err, "set_attach_target"))
> +		goto out;
> +
> +	err = bpf_object__load(obj);
> +	if (!ASSERT_OK(err, "obj_load"))
> +		goto out;
> +
> +	/* This will fail if the map is "owned" by a PROG_TYPE_EXT program,
> +	 * which, prior to fixing the kernel, it will be since the map is used
> +	 * from the 'redirect' prog above
> +	 */

The comment is confusing like 'which, prior to fixing the kernel, it 
will be'. IIUC, the verifier expects the map 'owner' program type is
PROG_TYPE_EXT, but it is XDP without this patch. Hence, the test will
fail without the patch 1.

> +	value.bpf_prog.fd = bpf_program__fd(drop);
> +	err = bpf_map_update_elem(bpf_map__fd(cpumap), &key, &value, 0);
> +	ASSERT_OK(err, "map_update");
> +
> +out:
> +	bpf_object__close(tgt_obj);
> +	bpf_object__close(obj);
> +}
> +
>   /* NOTE: affect other tests, must run in serial mode */
>   void serial_test_fexit_bpf2bpf(void)
>   {
> @@ -525,4 +576,6 @@ void serial_test_fexit_bpf2bpf(void)
>   		test_func_replace_global_func();
>   	if (test__start_subtest("fentry_to_cgroup_bpf"))
>   		test_fentry_to_cgroup_bpf();
> +	if (test__start_subtest("func_replace_progmap"))
> +		test_func_replace_progmap();
>   }
[...]