[PATCH v2 1/2] net: arp: collect context into new struct pending_arp

[PATCH v2 1/2] net: arp: collect context into new struct pending_arp

[Ahmad Fatoum]

The ARP code employs two global variables to communicate between the
code sending off the ARP request and the response that runs in the
poller:

- arp_wait_ip, which is the IP to be resolved
- arp_ether, which on success will point to the resulting MAC address

To make the relation between these two clearer and to prepare for the
follow-up fix, collect them into a common struct.

No functional change expected.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - use pending_arp.ip as correct loop condition
---
 net/net.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/net/net.c b/net/net.c
index 4703842f4377..795afed159aa 100644
--- a/net/net.c
+++ b/net/net.c
@@ -110,26 +110,33 @@ int setenv_ip(const char *name, IPaddr_t ip)
 	return 0;
 }
 
-static unsigned char *arp_ether;
-static IPaddr_t arp_wait_ip;
+/**
+ * struct pending_arp - Pending ARP state
+ * @ip: input IPv4 address whose resolution is being requested 
+ * @ether: output MAC addess buffer after receing a response
+ */
+static struct pending_arp {
+	IPaddr_t ip;
+	unsigned char *ether;
+} pending_arp;
 
 static void arp_handler(struct arprequest *arp)
 {
 	IPaddr_t tmp;
 
 	/* are we waiting for a reply */
-	if (!arp_wait_ip)
+	if (!pending_arp.ip)
 		return;
 
 	tmp = net_read_ip(&arp->ar_data[6]);
 
 	/* matched waiting packet's address */
-	if (tmp == arp_wait_ip) {
+	if (tmp == pending_arp.ip) {
 		/* save address for later use */
-		memcpy(arp_ether, &arp->ar_data[0], 6);
+		memcpy(pending_arp.ether, &arp->ar_data[0], 6);
 
 		/* no arp request pending now */
-		arp_wait_ip = 0;
+		pending_arp.ip = 0;
 	}
 }
 
@@ -162,6 +169,7 @@ static int arp_request(struct eth_device *edev, IPaddr_t dest, unsigned char *et
 	static char *arp_packet;
 	struct ethernet *et;
 	unsigned retries = 0;
+	IPaddr_t arp_wait_ip;
 	int ret;
 
 	if (!edev)
@@ -207,14 +215,15 @@ static int arp_request(struct eth_device *edev, IPaddr_t dest, unsigned char *et
 
 	net_write_ip(arp->ar_data + 16, arp_wait_ip);
 
-	arp_ether = ether;
+	pending_arp.ether = ether;
+	pending_arp.ip = arp_wait_ip;
 
 	ret = eth_send(edev, arp_packet, ETHER_HDR_SIZE + ARP_HDR_SIZE);
 	if (ret)
 		return ret;
 	arp_start = get_time_ns();
 
-	while (arp_wait_ip) {
+	while (pending_arp.ip) {
 		if (ctrlc())
 			return -EINTR;
 
-- 
2.47.3

[PATCH v2 2/2] net: reset pending ARP state when request is done

[Ahmad Fatoum]

net_new() creates a struct net_connection and calls arp_request()
to resolve unicast addresses. On success, arp_request() will
populate a buffer within the net_connection with the destination MAC
address.

If arp_request() aborts due to an error, it will leave the global
pending_arp.ether pointing at the buffer, which is promptly freed
leading to a dangling pointer and a use-after-free if we happen to get
an ARP response just after the error occurred.

Fix this memory safety issue by always clearing all of  pending_arp once
we are done with it, including error cases.

Reported-by: Sohaib Mohamed <sohaib.amhmd@gmail.com>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - no change
---
 net/net.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/net/net.c b/net/net.c
index 795afed159aa..9c5999604b67 100644
--- a/net/net.c
+++ b/net/net.c
@@ -220,24 +220,28 @@ static int arp_request(struct eth_device *edev, IPaddr_t dest, unsigned char *et
 
 	ret = eth_send(edev, arp_packet, ETHER_HDR_SIZE + ARP_HDR_SIZE);
 	if (ret)
-		return ret;
+		goto out;
 	arp_start = get_time_ns();
 
 	while (pending_arp.ip) {
-		if (ctrlc())
-			return -EINTR;
+		if (ctrlc()) {
+			ret = -EINTR;
+			goto out;
+		}
 
 		if (is_timeout(arp_start, 3 * SECOND)) {
 			printf("T ");
 			arp_start = get_time_ns();
 			ret = eth_send(edev, arp_packet, ETHER_HDR_SIZE + ARP_HDR_SIZE);
 			if (ret)
-				return ret;
+				goto out;
 			retries++;
 		}
 
-		if (retries > PKT_NUM_RETRIES)
-			return -ETIMEDOUT;
+		if (retries > PKT_NUM_RETRIES) {
+			ret = -ETIMEDOUT;
+			goto out;
+		}
 
 		net_poll();
 	}
@@ -245,7 +249,11 @@ static int arp_request(struct eth_device *edev, IPaddr_t dest, unsigned char *et
 	pr_debug("Got ARP REPLY for %pI4: %02x:%02x:%02x:%02x:%02x:%02x\n",
 		 &dest, ether[0], ether[1], ether[2], ether[3], ether[4],
 		 ether[5]);
-	return 0;
+
+out:
+	pending_arp.ip = 0;
+	pending_arp.ether = NULL;
+	return ret;
 }
 
 void net_poll(void)
-- 
2.47.3

Re: [PATCH v2 1/2] net: arp: collect context into new struct pending_arp

[Sascha Hauer]

On Fri, 28 Nov 2025 13:49:33 +0100, Ahmad Fatoum wrote:
> The ARP code employs two global variables to communicate between the
> code sending off the ARP request and the response that runs in the
> poller:
> 
> - arp_wait_ip, which is the IP to be resolved
> - arp_ether, which on success will point to the resulting MAC address
> 
> [...]

Applied, thanks!

[1/2] net: arp: collect context into new struct pending_arp
      https://git.pengutronix.de/cgit/barebox/commit/?id=8ed55472688a (link may not be stable)
[2/2] net: reset pending ARP state when request is done
      https://git.pengutronix.de/cgit/barebox/commit/?id=effaa57f9a94 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>