Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9840a4ff authored by Petar Penkov's avatar Petar Penkov Committed by Daniel Borkmann
Browse files

selftests/bpf: fix race in flow dissector tests 



Since the "last_dissection" map holds only the flow keys for the most
recent packet, there is a small race in the skb-less flow dissector
tests if a new packet comes between transmitting the test packet, and
reading its keys from the map. If this happens, the test packet keys
will be overwritten and the test will fail.

Changing the "last_dissection" map to a hash map, keyed on the
source/dest port pair resolves this issue. Additionally, let's clear the
last test results from the map between tests to prevent previous test
cases from interfering with the following test cases.

Fixes: 0905beec ("selftests/bpf: run flow dissector tests in skb-less mode")
Signed-off-by: default avatarPetar Penkov <ppenkov@google.com>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
parent d66fa3c7

tools/testing/selftests/bpf/prog_tests/flow_dissector.c

+21 −1
Original line number Diff line number Diff line
@@ -109,6 +109,8 @@ struct test tests[] = { 
			.iph.protocol = IPPROTO_TCP,

			.iph.tot_len = __bpf_constant_htons(MAGIC_BYTES),

			.tcp.doff = 5,

			.tcp.source = 80,

			.tcp.dest = 8080,

		},

		.keys = {

			.nhoff = ETH_HLEN,
@@ -116,6 +118,8 @@ struct test tests[] = { 
			.addr_proto = ETH_P_IP,

			.ip_proto = IPPROTO_TCP,

			.n_proto = __bpf_constant_htons(ETH_P_IP),

			.sport = 80,

			.dport = 8080,

		},

	},

	{
@@ -125,6 +129,8 @@ struct test tests[] = { 
			.iph.nexthdr = IPPROTO_TCP,

			.iph.payload_len = __bpf_constant_htons(MAGIC_BYTES),

			.tcp.doff = 5,

			.tcp.source = 80,

			.tcp.dest = 8080,

		},

		.keys = {

			.nhoff = ETH_HLEN,
@@ -132,6 +138,8 @@ struct test tests[] = { 
			.addr_proto = ETH_P_IPV6,

			.ip_proto = IPPROTO_TCP,

			.n_proto = __bpf_constant_htons(ETH_P_IPV6),

			.sport = 80,

			.dport = 8080,

		},

	},

	{
@@ -143,6 +151,8 @@ struct test tests[] = { 
			.iph.protocol = IPPROTO_TCP,

			.iph.tot_len = __bpf_constant_htons(MAGIC_BYTES),

			.tcp.doff = 5,

			.tcp.source = 80,

			.tcp.dest = 8080,

		},

		.keys = {

			.nhoff = ETH_HLEN + VLAN_HLEN,
@@ -150,6 +160,8 @@ struct test tests[] = { 
			.addr_proto = ETH_P_IP,

			.ip_proto = IPPROTO_TCP,

			.n_proto = __bpf_constant_htons(ETH_P_IP),

			.sport = 80,

			.dport = 8080,

		},

	},

	{
@@ -161,6 +173,8 @@ struct test tests[] = { 
			.iph.nexthdr = IPPROTO_TCP,

			.iph.payload_len = __bpf_constant_htons(MAGIC_BYTES),

			.tcp.doff = 5,

			.tcp.source = 80,

			.tcp.dest = 8080,

		},

		.keys = {

			.nhoff = ETH_HLEN + VLAN_HLEN * 2,
@@ -169,6 +183,8 @@ struct test tests[] = { 
			.addr_proto = ETH_P_IPV6,

			.ip_proto = IPPROTO_TCP,

			.n_proto = __bpf_constant_htons(ETH_P_IPV6),

			.sport = 80,

			.dport = 8080,

		},

	},

	{
@@ -487,7 +503,8 @@ void test_flow_dissector(void) 
			BPF_FLOW_DISSECTOR_F_PARSE_1ST_FRAG;

		struct bpf_prog_test_run_attr tattr = {};

		struct bpf_flow_keys flow_keys = {};

		__u32 key = 0;

		__u32 key = (__u32)(tests[i].keys.sport) << 16 |

			    tests[i].keys.dport;



		/* For skb-less case we can't pass input flags; run

		 * only the tests that have a matching set of flags.
@@ -504,6 +521,9 @@ void test_flow_dissector(void) 


		CHECK_ATTR(err, tests[i].name, "skb-less err %d\n", err);

		CHECK_FLOW_KEYS(tests[i].name, flow_keys, tests[i].keys);



		err = bpf_map_delete_elem(keys_fd, &key);

		CHECK_ATTR(err, tests[i].name, "bpf_map_delete_elem %d\n", err);

	}



	bpf_prog_detach(prog_fd, BPF_FLOW_DISSECTOR);

tools/testing/selftests/bpf/progs/bpf_flow.c

+6 −7
Original line number Diff line number Diff line
@@ -65,8 +65,8 @@ struct { 
} jmp_table SEC(".maps");



struct {

	__uint(type, BPF_MAP_TYPE_ARRAY);

	__uint(max_entries, 1);

	__uint(type, BPF_MAP_TYPE_HASH);

	__uint(max_entries, 1024);

	__type(key, __u32);

	__type(value, struct bpf_flow_keys);

} last_dissection SEC(".maps");
@@ -74,12 +74,11 @@ struct { 
static __always_inline int export_flow_keys(struct bpf_flow_keys *keys,

					    int ret)

{

	struct bpf_flow_keys *val;

	__u32 key = 0;

	__u32 key = (__u32)(keys->sport) << 16 | keys->dport;

	struct bpf_flow_keys val;



	val = bpf_map_lookup_elem(&last_dissection, &key);

	if (val)

		memcpy(val, keys, sizeof(*val));

	memcpy(&val, keys, sizeof(val));

	bpf_map_update_elem(&last_dissection, &key, &val, BPF_ANY);

	return ret;

}