From 55802e5e8637949b113a9f225c9f9a11f7f9b49d Mon Sep 17 00:00:00 2001 From: Krishna T Date: Sat, 18 Feb 2023 02:07:54 +0530 Subject: [PATCH] net: l2: ethernet: Fix double free In the case of no ARP entry, the incoming packet is added to the ARP's pending queue, while ARP is being resolved. Here a reference is taken by the ARP layer to the packet to avoid it being freed, but the Ethernet immediately puts down the reference and send the ARP packet to the driver. If the ARP request fails for some reason, L2 returns failure to net_if which then puts down the reference and the packet will be freed as the reference count is now zero. But the packet is still in the ARP's pending queue and after timeout ARP will put down the reference causing double free bus fault (double free message is only seen if the CONFIG_NET_PKT_LOG_LEVEL_DBG is enabled, so, a bit hard to debug. Fix this by clearing the ARP entry and pending queue after taking a reference and then free ARP packet, IP packets are either freed by ARP pending queue drain or net_if layer. Signed-off-by: Krishna T --- subsys/net/l2/ethernet/arp.c | 13 +++++++++++++ subsys/net/l2/ethernet/arp.h | 4 ++++ subsys/net/l2/ethernet/ethernet.c | 16 +++++++++++++++- 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/subsys/net/l2/ethernet/arp.c b/subsys/net/l2/ethernet/arp.c index d89b3b723d..cf2f596eff 100644 --- a/subsys/net/l2/ethernet/arp.c +++ b/subsys/net/l2/ethernet/arp.c @@ -756,6 +756,19 @@ void net_arp_clear_cache(struct net_if *iface) k_mutex_unlock(&arp_mutex); } +int net_arp_clear_pending(struct net_if *iface, struct in_addr *dst) +{ + struct arp_entry *entry = arp_entry_find_pending(iface, dst); + + if (!entry) { + return -ENOENT; + } + + arp_entry_cleanup(entry, true); + + return 0; +} + int net_arp_foreach(net_arp_cb_t cb, void *user_data) { int ret = 0; diff --git a/subsys/net/l2/ethernet/arp.h b/subsys/net/l2/ethernet/arp.h index ceb46ac985..28cafe5f20 100644 --- a/subsys/net/l2/ethernet/arp.h +++ b/subsys/net/l2/ethernet/arp.h @@ -49,6 +49,9 @@ struct net_pkt *net_arp_prepare(struct net_pkt *pkt, enum net_verdict net_arp_input(struct net_pkt *pkt, struct net_eth_hdr *eth_hdr); +int net_arp_clear_pending(struct net_if *iface, + struct in_addr *dst); + struct arp_entry { sys_snode_t node; uint32_t req_start; @@ -79,6 +82,7 @@ void net_arp_init(void); #define net_arp_clear_cache(...) #define net_arp_foreach(...) 0 #define net_arp_init(...) +#define net_arp_clear_pending(...) 0 #endif /* CONFIG_NET_ARP */ diff --git a/subsys/net/l2/ethernet/ethernet.c b/subsys/net/l2/ethernet/ethernet.c index 4bcd892e43..4fdd462e4c 100644 --- a/subsys/net/l2/ethernet/ethernet.c +++ b/subsys/net/l2/ethernet/ethernet.c @@ -600,8 +600,9 @@ static int ethernet_send(struct net_if *iface, struct net_pkt *pkt) { const struct ethernet_api *api = net_if_get_device(iface)->api; struct ethernet_context *ctx = net_if_l2_data(iface); - uint16_t ptype; + uint16_t ptype = 0; int ret; + struct net_pkt *orig_pkt = pkt; if (!api) { ret = -ENOENT; @@ -716,6 +717,19 @@ send: if (ret != 0) { eth_stats_update_errors_tx(iface); ethernet_remove_l2_header(pkt); + if (IS_ENABLED(CONFIG_NET_ARP) && ptype == htons(NET_ETH_PTYPE_ARP)) { + /* Original packet was added to ARP's pending Q, so, to avoid it + * being freed, take a reference, the reference is dropped when we + * clear the pending Q in ARP and then it will be freed by net_if. + */ + net_pkt_ref(orig_pkt); + if (net_arp_clear_pending(iface, + (struct in_addr *)NET_IPV4_HDR(pkt)->dst)) { + NET_DBG("Could not find pending ARP entry"); + } + /* Free the ARP request */ + net_pkt_unref(pkt); + } goto error; }