net: arp: Fix ARP protocol handler to not use Ethernet hdr directly

The ARP protocol handler cannot directly access the Ethernet header
because the caller has removed the header already when the handler
is called. So change net_arp_input() and pass source and destination
MAC address there instead of bogus pointer that was pointing to ARP
header instead of Ethernet header. This requires changes to ARP tests.

Signed-off-by: Jukka Rissanen <[email protected]>

Author: jukkar

subsys/net/l2/ethernet/arp.c

@@ -718,7 +718,6 @@ void net_arp_update(struct net_if *iface,
 
 static inline struct net_pkt *arp_prepare_reply(struct net_if *iface,
 						struct net_pkt *req,
-						struct net_eth_hdr *eth_query,
 						struct net_eth_addr *dst_addr)
 {
 	struct net_arp_hdr *hdr, *query;
@@ -780,18 +779,17 @@ static bool arp_hdr_check(struct net_arp_hdr *arp_hdr)
 }
 
 enum net_verdict net_arp_input(struct net_pkt *pkt,
-			       struct net_eth_hdr *eth_hdr)
+			       struct net_eth_addr *src,
+			       struct net_eth_addr *dst)
 {
 	struct net_eth_addr *dst_hw_addr;
 	struct net_arp_hdr *arp_hdr;
 	struct net_pkt *reply;
 	struct in_addr *addr;
 
-	if (net_pkt_get_len(pkt) < (sizeof(struct net_arp_hdr) -
-				    (net_pkt_ip_data(pkt) - (uint8_t *)eth_hdr))) {
-		NET_DBG("Invalid ARP header (len %zu, min %zu bytes) %p",
-			net_pkt_get_len(pkt), sizeof(struct net_arp_hdr) -
-			(net_pkt_ip_data(pkt) - (uint8_t *)eth_hdr), pkt);
+	if (net_pkt_get_len(pkt) < sizeof(struct net_arp_hdr)) {
+		NET_DBG("DROP: Too short ARP msg (%zu bytes, min %zu bytes)",
+			net_pkt_get_len(pkt), sizeof(struct net_arp_hdr));
 		return NET_DROP;
 	}
 
@@ -812,7 +810,7 @@ enum net_verdict net_arp_input(struct net_pkt *pkt,
 		}
 
 		if (IS_ENABLED(CONFIG_NET_ARP_GRATUITOUS)) {
-			if (net_eth_is_addr_broadcast(&eth_hdr->dst) &&
+			if (net_eth_is_addr_broadcast(dst) &&
 			    (net_eth_is_addr_broadcast(&arp_hdr->dst_hwaddr) ||
 			     net_eth_is_addr_all_zeroes(&arp_hdr->dst_hwaddr)) &&
 			    net_ipv4_addr_cmp_raw(arp_hdr->dst_ipaddr,
@@ -831,7 +829,7 @@ enum net_verdict net_arp_input(struct net_pkt *pkt,
 		/* Discard ARP request if Ethernet address is broadcast
 		 * and Source IP address is Multicast address.
 		 */
-		if (memcmp(&eth_hdr->dst, net_eth_broadcast_addr(),
+		if (memcmp(dst, net_eth_broadcast_addr(),
 			   sizeof(struct net_eth_addr)) == 0 &&
 		    net_ipv4_is_addr_mcast((struct in_addr *)arp_hdr->src_ipaddr)) {
 			NET_DBG("DROP: eth addr is bcast, src addr is mcast");
@@ -870,12 +868,11 @@ enum net_verdict net_arp_input(struct net_pkt *pkt,
 
 			dst_hw_addr = &arp_hdr->src_hwaddr;
 		} else {
-			dst_hw_addr = &eth_hdr->src;
+			dst_hw_addr = src;
 		}
 
 		/* Send reply */
-		reply = arp_prepare_reply(net_pkt_iface(pkt), pkt, eth_hdr,
-					  dst_hw_addr);
+		reply = arp_prepare_reply(net_pkt_iface(pkt), pkt, dst_hw_addr);
 		if (reply) {
 			net_if_queue_tx(net_pkt_iface(reply), reply);
 		} else {
@@ -1022,23 +1019,23 @@ static enum net_verdict arp_recv(struct net_if *iface,
 				 uint16_t ptype,
 				 struct net_pkt *pkt)
 {
-	struct net_eth_hdr *hdr = NET_ETH_HDR(pkt);
-
 	ARG_UNUSED(iface);
 	ARG_UNUSED(ptype);
 
 	net_pkt_set_family(pkt, AF_INET);
 
 	NET_DBG("ARP packet from %s received",
-		net_sprint_ll_addr((uint8_t *)hdr->src.addr,
+		net_sprint_ll_addr(net_pkt_lladdr_src(pkt)->addr,
 				   sizeof(struct net_eth_addr)));
 
 	if (IS_ENABLED(CONFIG_NET_IPV4_ACD) &&
 	    net_ipv4_acd_input(iface, pkt) == NET_DROP) {
 		return NET_DROP;
 	}
 
-	return net_arp_input(pkt, hdr);
+	return net_arp_input(pkt,
+			     (struct net_eth_addr *)net_pkt_lladdr_src(pkt)->addr,
+			     (struct net_eth_addr *)net_pkt_lladdr_dst(pkt)->addr);
 }
 
 ETH_NET_L3_REGISTER(ARP, NET_ETH_PTYPE_ARP, arp_recv);

subsys/net/l2/ethernet/arp.h

@@ -47,7 +47,8 @@ struct net_pkt *net_arp_prepare(struct net_pkt *pkt,
 				struct in_addr *request_ip,
 				struct in_addr *current_ip);
 enum net_verdict net_arp_input(struct net_pkt *pkt,
-			       struct net_eth_hdr *eth_hdr);
+			       struct net_eth_addr *src,
+			       struct net_eth_addr *dst);
 
 int net_arp_clear_pending(struct net_if *iface,
 				struct in_addr *dst);

tests/net/arp/src/main.c

@@ -313,6 +313,7 @@ ZTEST(arp_fn_tests, test_arp)
 	}
 
 	struct net_eth_hdr *eth_hdr = NULL;
+	struct net_eth_addr dst_lladdr;
 	struct net_pkt *pkt;
 	struct net_pkt *pkt2;
 	struct net_if *iface;
@@ -330,6 +331,8 @@ ZTEST(arp_fn_tests, test_arp)
 
 	net_arp_init();
 
+	(void)memset(&dst_lladdr, 0xff, sizeof(struct net_eth_addr));
+
 	iface = net_if_lookup_by_dev(DEVICE_GET(net_arp_test));
 
 	net_if_ipv4_set_gw(iface, &gw);
@@ -528,7 +531,9 @@ ZTEST(arp_fn_tests, test_arp)
 	zassert_not_null(pkt2, "ARP reply generation failed.");
 
 	/* The pending packet should now be sent */
-	switch (net_arp_input(pkt2, eth_hdr)) {
+	switch (net_arp_input(pkt2,
+			      (struct net_eth_addr *)net_pkt_lladdr_src(pkt2)->addr,
+			      &dst_lladdr)) {
 	case NET_OK:
 	case NET_CONTINUE:
 		break;
@@ -573,7 +578,9 @@ ZTEST(arp_fn_tests, test_arp)
 
 	req_test = true;
 
-	switch (net_arp_input(pkt2, eth_hdr)) {
+	switch (net_arp_input(pkt2,
+			      (struct net_eth_addr *)net_pkt_lladdr_src(pkt2)->addr,
+			      &dst_lladdr)) {
 	case NET_OK:
 	case NET_CONTINUE:
 		break;
@@ -630,7 +637,10 @@ ZTEST(arp_fn_tests, test_arp)
 
 		net_pkt_set_ll_proto_type(pkt, NET_ETH_PTYPE_ARP);
 
-		verdict = net_arp_input(pkt, eth_hdr);
+		verdict = net_arp_input(pkt,
+					(struct net_eth_addr *)net_pkt_lladdr_src(pkt)->addr,
+					&dst_lladdr);
+
 		zassert_not_equal(verdict, NET_DROP, "Gratuitous ARP failed");
 
 		/* Then check that the HW address is changed for an existing