From a714b420807d09d9c073bc9fc608235cd6dcb435 Mon Sep 17 00:00:00 2001 From: Kieran Tyrrell Date: Mon, 2 Oct 2023 18:13:34 +0100 Subject: [PATCH] net: ip: fix promiscuous mode altering packets Always clone net_pkt to pass to promiscuous queue. Previously, net_pkt was passed to L2 before conditionally cloning. But L2 would in some cases strip ethernet headers, so cloned net_pkt received through promiscuous interface would be missing headers. Signed-off-by: Kieran Tyrrell --- subsys/net/ip/net_if.c | 38 +++----------------------------- tests/net/promiscuous/src/main.c | 20 +++++++++++++++-- 2 files changed, 21 insertions(+), 37 deletions(-) diff --git a/subsys/net/ip/net_if.c b/subsys/net/ip/net_if.c index bfa9c3cfdf7ed6..2b084de071fa6a 100644 --- a/subsys/net/ip/net_if.c +++ b/subsys/net/ip/net_if.c @@ -4120,45 +4120,13 @@ enum net_verdict net_if_recv_data(struct net_if *iface, struct net_pkt *pkt) { if (IS_ENABLED(CONFIG_NET_PROMISCUOUS_MODE) && net_if_is_promisc(iface)) { - /* If the packet is not for us and the promiscuous - * mode is enabled, then increase the ref count so - * that net_core.c:processing_data() will not free it. - * The promiscuous mode handler must free the packet - * after it has finished working with it. - * - * If packet is for us, then NET_CONTINUE is returned. - * In this case we must clone the packet, as the packet - * could be manipulated by other part of the stack. - */ - enum net_verdict verdict; struct net_pkt *new_pkt; - /* This protects pkt so that it will not be freed by L2 recv() - */ - net_pkt_ref(pkt); + new_pkt = net_pkt_clone(pkt, K_NO_WAIT); - verdict = net_if_l2(iface)->recv(iface, pkt); - if (verdict == NET_CONTINUE) { - new_pkt = net_pkt_clone(pkt, K_NO_WAIT); - } else { - new_pkt = net_pkt_ref(pkt); + if (net_promisc_mode_input(new_pkt) == NET_DROP) { + net_pkt_unref(new_pkt); } - - /* L2 has modified the buffer starting point, it is easier - * to re-initialize the cursor rather than updating it. - */ - if (new_pkt) { - net_pkt_cursor_init(new_pkt); - - if (net_promisc_mode_input(new_pkt) == NET_DROP) { - net_pkt_unref(new_pkt); - } - } else { - NET_WARN("promiscuous packet dropped, unable to clone packet"); - } - net_pkt_unref(pkt); - - return verdict; } return net_if_l2(iface)->recv(iface, pkt); diff --git a/tests/net/promiscuous/src/main.c b/tests/net/promiscuous/src/main.c index 0b6e6a4795714b..9bedd9773b916c 100644 --- a/tests/net/promiscuous/src/main.c +++ b/tests/net/promiscuous/src/main.c @@ -352,6 +352,8 @@ static void _recv_data(struct net_if *iface, struct net_pkt **pkt) *pkt = net_pkt_rx_alloc_with_buffer(iface, sizeof(data), AF_UNSPEC, 0, K_FOREVER); + net_pkt_ref(*pkt); + net_pkt_write(*pkt, data, sizeof(data)); ret = net_recv_data(iface, *pkt); @@ -372,11 +374,25 @@ static void test_verify_data(void) struct net_pkt *pkt; pkt = net_promisc_mode_wait_data(K_SECONDS(1)); - zassert_equal_ptr(pkt, pkt1, "pkt %p != %p", pkt, pkt1); + zassert_not_null(pkt, "pkt"); + zassert_not_null(pkt->buffer, "pkt->buffer"); + zassert_not_null(pkt1, "pkt1"); + zassert_not_null(pkt1->buffer, "pkt1->buffer"); + zassert_equal(pkt->buffer->len, pkt1->buffer->len, "packet length differs"); + zassert_not_null(pkt->buffer->data, "pkt->buffer->data"); + zassert_not_null(pkt1->buffer->data, "pkt1->buffer->data"); + zassert_mem_equal(pkt->buffer->data, pkt1->buffer->data, pkt1->buffer->len); net_pkt_unref(pkt); pkt = net_promisc_mode_wait_data(K_SECONDS(1)); - zassert_equal_ptr(pkt, pkt2, "pkt %p != %p", pkt, pkt2); + zassert_not_null(pkt, "pkt"); + zassert_not_null(pkt->buffer, "pkt->buffer"); + zassert_not_null(pkt2, "pkt2"); + zassert_not_null(pkt2->buffer, "pkt2->buffer"); + zassert_equal(pkt->buffer->len, pkt2->buffer->len, "packet length differs"); + zassert_not_null(pkt->buffer->data, "pkt->buffer->data"); + zassert_not_null(pkt2->buffer->data, "pkt2->buffer->data"); + zassert_mem_equal(pkt->buffer->data, pkt2->buffer->data, pkt2->buffer->len); net_pkt_unref(pkt); }