kern/58297: net/nd.c leaks mbufs pending neighbour discovery

[campbell+netbsd%mumble.net@localhost]

>Number:         58297
>Category:       kern
>Synopsis:       net/nd.c leaks mbufs pending neighbour discovery
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue May 28 21:45:01 +0000 2024
>Originator:     Taylor R Campbell
>Release:        current, 10
>Organization:
The NDBSD Foundation
>Environment:
>Description:
When the network stack needs the link-layer address of the next hop for an outgoing packet, it asks nd_resolve (e.g., via ether_output -> arpresolve).

If nd_resolve doesn't know the link-layer address, it queues up the packet's mbuf in struct llentry::la_hold (a.k.a. ln_hold) for all packets to be transmitted via the same next hop once the link-layer address is resolved (via arp, ipv6nd, ...).  (If the queue exceeds a maximum length -- currently, always 1 -- then older packets are freed.)

From time to time, the still-pending packets are supposed to be freed in lltable_drop_entry_queue, via lltable_drain (from arp_drain, called by arp_fasttimo; XXX no IPv6 callers?) or via llentry_free (from lltable_purge_entries, called by in_if_down, in6_if_down, nd6_purge).

But lltable_drop_entry_queue only frees the queued mbufs if la_numheld is positive.  And nothing ever increments la_numheld.  So whenever an llentry is freed, e.g. when an interface is brought down, it will leak any queued mbufs waiting for their next hop's link-layer address resolution.

Found by Edgar Fuss: https://mail-index.netbsd.org/tech-net/2024/04/23/msg008762.html
>How-To-Repeat:
1. ping a host on the local network that doesn't exist
2. bring the interface down before link-layer resolution has timed out and given up
>Fix:
From 2786a8a8ed3b3223888c8374c5d32162866aec91 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Wed, 24 Apr 2024 14:08:35 +0000
Subject: [PATCH] nd_resolve: Maintain la_numheld.

Otherwise lltable_drop_entry_quene never drops anything.

Addresses mbuf leak, PR kern/XXXXX.
---
 sys/net/nd.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/sys/net/nd.c b/sys/net/nd.c
index c9c2ca6fddec..7be385b74693 100644
--- a/sys/net/nd.c
+++ b/sys/net/nd.c
@@ -374,14 +374,25 @@ nd_resolve(struct llentry *ln, const struct rtentry *rt, struct mbuf *m,
 				break;
 			}
 		}
+		KASSERTMSG(ln->la_numheld == i, "la_numheld=%d i=%d",
+		    ln->la_numheld, i);
 		while (i >= nd->nd_maxqueuelen) {
 			m_hold = ln->ln_hold;
 			ln->ln_hold = ln->ln_hold->m_nextpkt;
 			m_freem(m_hold);
 			i--;
+			ln->la_numheld--;
 		}
-	} else
+	} else {
+		KASSERTMSG(ln->la_numheld == 0, "la_numheld=%d",
+		    ln->la_numheld);
 		ln->ln_hold = m;
+	}
+
+	KASSERTMSG(ln->la_numheld < nd->nd_maxqueuelen,
+	    "la_numheld=%d nd_maxqueuelen=%d",
+	    ln->la_numheld, nd->nd_maxqueuelen);
+	ln->la_numheld++;
 
 	if (ln->ln_asked >= nd->nd_mmaxtries)
 		error = (rt != NULL && rt->rt_flags & RTF_GATEWAY) ?