Re: [PATCH bpf v3 2/2] selftests/bpf: Cover generic devmap egress last-dst rewrite

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Uninitialized `net_config` is passed to `cleanup_network()` on early error paths, causing arbitrary shell commands to be executed via `system()`.
--

commit 60bbc3bd1e74368d2d682d8956f86b733750dfd9
Author: Sun Jian <sun.jian.kdev@gmail.com>
selftests/bpf: Cover generic devmap egress last-dst rewrite

This patch strengthens xdp_veth_egress to check that destinations observe
the correct MAC address for their own egress ifindex. It also adds a new
generic XDP last-destination test to cover cases where earlier destinations
do not have a devmap egress program.

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_xdp_veth.c b/tools/testing/selftests/bpf/prog_tests/test_xdp_veth.c
> index 3e98a16659362..681415cacb75c 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_xdp_veth.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_xdp_veth.c
[ ... ]
> +static void xdp_veth_egress_last_dst(u32 flags)
> +{
[ ... ]
> +	struct net_configuration net_config;
> +	int mac_map, egress_map, res_map;
> +	struct nstoken *nstoken = NULL;
> +	struct xdp_dummy *xdp_dummy;
> +	__be64 last_mac = 0;
> +	bool found = false;
> +	int err;
> +	int i;
> +
> +	xdp_dummy = xdp_dummy__open_and_load();
> +	if (!ASSERT_OK_PTR(xdp_dummy, "xdp_dummy__open_and_load"))
> +		return;
> +
> +	xdp_redirect_multi_kern = xdp_redirect_multi_kern__open_and_load();
> +	if (!ASSERT_OK_PTR(xdp_redirect_multi_kern, "xdp_redirect_multi_kern__open_and_load"))
> +		goto destroy_xdp_dummy;
> +
> +	xdp_redirect_map = xdp_redirect_map__open_and_load();
> +	if (!ASSERT_OK_PTR(xdp_redirect_map, "xdp_redirect_map__open_and_load"))
> +		goto destroy_xdp_redirect_multi_kern;
> +
> +	if (!ASSERT_OK(create_network(&net_config), "create network"))
> +		goto destroy_xdp_redirect_map;
[ ... ]
> +destroy_xdp_redirect_map:
> +	close_netns(nstoken);
> +	xdp_redirect_map__destroy(xdp_redirect_map);
> +destroy_xdp_redirect_multi_kern:
> +	xdp_redirect_multi_kern__destroy(xdp_redirect_multi_kern);
> +destroy_xdp_dummy:
> +	xdp_dummy__destroy(xdp_dummy);
> +
> +	cleanup_network(&net_config);

[Severity: High]
This isn't a bug introduced by this patch, but does this early error
handling path pass uninitialized stack memory to cleanup_network()?

If xdp_redirect_multi_kern__open_and_load() or
xdp_redirect_map__open_and_load() fail before create_network() is called,
the goto labels eventually reach cleanup_network(&net_config).

Since cleanup_network() passes net_config fields like ns0_name to SYS_NOFAIL
which executes system commands, could passing uninitialized stack garbage
here lead to unpredictable shell command execution as root?

Would it be safer to zero-initialize net_config with `= {}` when declaring it?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611043317.512843-1-sun.jian.kdev@gmail.com?part=2