From a2b08c94260a7cb2d6284c2c51828913fb93553f Mon Sep 17 00:00:00 2001 From: Stanislav Fomichev Date: Fri, 8 Sep 2023 14:00:06 -0700 Subject: [PATCH 1/2] bpf: return correct -ENOBUFS from bpf_clone_redirect Commit 151e887d8ff9 ("veth: Fixing transmit return status for dropped packets") exposed the fact that bpf_clone_redirect is capable of returning raw NET_XMIT_XXX return codes. This is in the conflict with its UAPI doc which says the following: "0 on success, or a negative error in case of failure." Let's wrap dev_queue_xmit's return value (in __bpf_tx_skb) into net_xmit_errno to make sure we correctly propagate NET_XMIT_DROP as -ENOBUFS instead of 1. Note, this is technically breaking existing UAPI where we used to return 1 and now will do -ENOBUFS. The alternative is to document that bpf_clone_redirect can return 1 for DROP and 2 for CN. Reported-by: Daniel Borkmann Signed-off-by: Stanislav Fomichev --- net/core/filter.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/net/core/filter.c b/net/core/filter.c index a094694899c99..9e297931b02f4 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -2129,6 +2129,9 @@ static inline int __bpf_tx_skb(struct net_device *dev, struct sk_buff *skb) ret = dev_queue_xmit(skb); dev_xmit_recursion_dec(); + if (ret > 0) + ret = net_xmit_errno(ret); + return ret; } From 4a8a7510b5f70f3c38ae8ce4c9a7058f696caa5e Mon Sep 17 00:00:00 2001 From: Stanislav Fomichev Date: Fri, 8 Sep 2023 14:00:07 -0700 Subject: [PATCH 2/2] selftests/bpf: update bpf_clone_redirect expected return code Commit 151e887d8ff9 ("veth: Fixing transmit return status for dropped packets") started propagating proper NET_XMIT_DROP error into the caller which means it's now possible to get -ENOBUFS when calling bpf_clone_redirect() in this particular test. Update the test to reflect that. Reported-by: Daniel Borkmann Signed-off-by: Stanislav Fomichev --- tools/testing/selftests/bpf/prog_tests/empty_skb.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/empty_skb.c b/tools/testing/selftests/bpf/prog_tests/empty_skb.c index 3b77d8a422dbf..b9f5cb3120336 100644 --- a/tools/testing/selftests/bpf/prog_tests/empty_skb.c +++ b/tools/testing/selftests/bpf/prog_tests/empty_skb.c @@ -24,6 +24,7 @@ void test_empty_skb(void) int *ifindex; int err; int ret; + int lwt_egress_ret; /* expected retval at lwt/egress */ bool success_on_tc; } tests[] = { /* Empty packets are always rejected. */ @@ -57,6 +58,7 @@ void test_empty_skb(void) .data_size_in = sizeof(eth_hlen), .ifindex = &veth_ifindex, .ret = -ERANGE, + .lwt_egress_ret = -ERANGE, .success_on_tc = true, }, { @@ -70,6 +72,7 @@ void test_empty_skb(void) .data_size_in = sizeof(eth_hlen), .ifindex = &ipip_ifindex, .ret = -ERANGE, + .lwt_egress_ret = -ERANGE, }, /* ETH_HLEN+1-sized packet should be redirected. */ @@ -79,6 +82,7 @@ void test_empty_skb(void) .data_in = eth_hlen_pp, .data_size_in = sizeof(eth_hlen_pp), .ifindex = &veth_ifindex, + .lwt_egress_ret = -ENOBUFS, }, { .msg = "ipip ETH_HLEN+1 packet ingress", @@ -108,8 +112,12 @@ void test_empty_skb(void) for (i = 0; i < ARRAY_SIZE(tests); i++) { bpf_object__for_each_program(prog, bpf_obj->obj) { - char buf[128]; + bool at_egress = strstr(bpf_program__name(prog), "egress") != NULL; bool at_tc = !strncmp(bpf_program__section_name(prog), "tc", 2); + int expected_ret; + char buf[128]; + + expected_ret = at_egress && !at_tc ? tests[i].lwt_egress_ret : tests[i].ret; tattr.data_in = tests[i].data_in; tattr.data_size_in = tests[i].data_size_in; @@ -128,7 +136,7 @@ void test_empty_skb(void) if (at_tc && tests[i].success_on_tc) ASSERT_GE(bpf_obj->bss->ret, 0, buf); else - ASSERT_EQ(bpf_obj->bss->ret, tests[i].ret, buf); + ASSERT_EQ(bpf_obj->bss->ret, expected_ret, buf); } }