git: d18b4bec98f1 - main - netinet6: Fix mbuf leak in NDP
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Tue, 31 May 2022 21:06:49 UTC
The branch main has been updated by melifaro:
URL: https://cgit.FreeBSD.org/src/commit/?id=d18b4bec98f1cf3c51103a22c0c041e6238c44c7
commit d18b4bec98f1cf3c51103a22c0c041e6238c44c7
Author: Arseny Smalyuk <smalukav@gmail.com>
AuthorDate: 2022-05-31 20:04:51 +0000
Commit: Alexander V. Chernikov <melifaro@FreeBSD.org>
CommitDate: 2022-05-31 21:06:14 +0000
netinet6: Fix mbuf leak in NDP
Mbufs leak when manually removing incomplete NDP records with pending packet via ndp -d.
It happens because lltable_drop_entry_queue() rely on `la_numheld`
counter when dropping NDP entries (lles). It turned out NDP code never
increased `la_numheld`, so the actual free never happened.
Fix the issue by introducing unified lltable_append_entry_queue(),
common for both ARP and NDP code, properly addressing packet queue
maintenance.
Reviewed By: melifaro
Differential Revision: https://reviews.freebsd.org/D35365
MFC after: 2 weeks
---
sys/net/if_llatbl.c | 43 ++++++++++++++++++++++++++++++++++-----
sys/net/if_llatbl.h | 2 ++
sys/netinet/icmp6.h | 1 +
sys/netinet/if_ether.c | 23 ++++-----------------
sys/netinet6/nd6.c | 53 ++++++++++---------------------------------------
usr.bin/netstat/inet6.c | 2 ++
6 files changed, 58 insertions(+), 66 deletions(-)
diff --git a/sys/net/if_llatbl.c b/sys/net/if_llatbl.c
index b7ea4bc8e4ea..7d05effb4d66 100644
--- a/sys/net/if_llatbl.c
+++ b/sys/net/if_llatbl.c
@@ -127,6 +127,41 @@ done:
return (error);
}
+/*
+ * Adds a mbuf to hold queue. Drops old packets if the queue is full.
+ *
+ * Returns the number of held packets that were dropped.
+ */
+size_t
+lltable_append_entry_queue(struct llentry *lle, struct mbuf *m,
+ size_t maxheld)
+{
+ size_t pkts_dropped = 0;
+
+ LLE_WLOCK_ASSERT(lle);
+
+ while (lle->la_numheld >= maxheld && lle->la_hold != NULL) {
+ struct mbuf *next = lle->la_hold->m_nextpkt;
+ m_freem(lle->la_hold);
+ lle->la_hold = next;
+ lle->la_numheld--;
+ pkts_dropped++;
+ }
+
+ if (lle->la_hold != NULL) {
+ struct mbuf *curr = lle->la_hold;
+ while (curr->m_nextpkt != NULL)
+ curr = curr->m_nextpkt;
+ curr->m_nextpkt = m;
+ } else
+ lle->la_hold = m;
+
+ lle->la_numheld++;
+
+ return pkts_dropped;
+}
+
+
/*
* Common function helpers for chained hash table.
*/
@@ -285,14 +320,12 @@ llentries_unlink(struct lltable *llt, struct llentries *head)
size_t
lltable_drop_entry_queue(struct llentry *lle)
{
- size_t pkts_dropped;
- struct mbuf *next;
+ size_t pkts_dropped = 0;
LLE_WLOCK_ASSERT(lle);
- pkts_dropped = 0;
- while ((lle->la_numheld > 0) && (lle->la_hold != NULL)) {
- next = lle->la_hold->m_nextpkt;
+ while (lle->la_hold != NULL) {
+ struct mbuf *next = lle->la_hold->m_nextpkt;
m_freem(lle->la_hold);
lle->la_hold = next;
lle->la_numheld--;
diff --git a/sys/net/if_llatbl.h b/sys/net/if_llatbl.h
index 5c5dd7a9e08f..36f4670f6bd5 100644
--- a/sys/net/if_llatbl.h
+++ b/sys/net/if_llatbl.h
@@ -231,6 +231,8 @@ void lltable_link(struct lltable *llt);
void lltable_prefix_free(int, struct sockaddr *,
struct sockaddr *, u_int);
int lltable_sysctl_dumparp(int, struct sysctl_req *);
+size_t lltable_append_entry_queue(struct llentry *,
+ struct mbuf *, size_t);
struct lltable *in_lltable_get(struct ifnet *ifp);
struct lltable *in6_lltable_get(struct ifnet *ifp);
diff --git a/sys/netinet/icmp6.h b/sys/netinet/icmp6.h
index d4fde01c7e88..d4a103d04f00 100644
--- a/sys/netinet/icmp6.h
+++ b/sys/netinet/icmp6.h
@@ -602,6 +602,7 @@ struct icmp6stat {
uint64_t icp6s_tooshort; /* packet < sizeof(struct icmp6_hdr) */
uint64_t icp6s_checksum; /* bad checksum */
uint64_t icp6s_badlen; /* calculated bound mismatch */
+ uint64_t icp6s_dropped; /* # of packets dropped waiting for a resolution */
/*
* number of responses: this member is inherited from netinet code, but
* for netinet6 code, it is already available in icp6s_outhist[].
diff --git a/sys/netinet/if_ether.c b/sys/netinet/if_ether.c
index 380f8fde35c5..1c6c35f180aa 100644
--- a/sys/netinet/if_ether.c
+++ b/sys/netinet/if_ether.c
@@ -464,8 +464,6 @@ arpresolve_full(struct ifnet *ifp, int is_gw, int flags, struct mbuf *m,
struct llentry **plle)
{
struct llentry *la = NULL, *la_tmp;
- struct mbuf *curr = NULL;
- struct mbuf *next = NULL;
int error, renew;
char *lladdr;
int ll_len;
@@ -533,6 +531,7 @@ arpresolve_full(struct ifnet *ifp, int is_gw, int flags, struct mbuf *m,
}
renew = (la->la_asked == 0 || la->la_expire != time_uptime);
+
/*
* There is an arptab entry, but no ethernet address
* response yet. Add the mbuf to the list, dropping
@@ -540,24 +539,10 @@ arpresolve_full(struct ifnet *ifp, int is_gw, int flags, struct mbuf *m,
* setting.
*/
if (m != NULL) {
- if (la->la_numheld >= V_arp_maxhold) {
- if (la->la_hold != NULL) {
- next = la->la_hold->m_nextpkt;
- m_freem(la->la_hold);
- la->la_hold = next;
- la->la_numheld--;
- ARPSTAT_INC(dropped);
- }
- }
- if (la->la_hold != NULL) {
- curr = la->la_hold;
- while (curr->m_nextpkt != NULL)
- curr = curr->m_nextpkt;
- curr->m_nextpkt = m;
- } else
- la->la_hold = m;
- la->la_numheld++;
+ size_t dropped = lltable_append_entry_queue(la, m, V_arp_maxhold);
+ ARPSTAT_ADD(dropped, dropped);
}
+
/*
* Return EWOULDBLOCK if we have tried less than arp_maxtries. It
* will be masked by ether_output(). Return EHOSTDOWN/EHOSTUNREACH
diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c
index 69da61aa9c5c..efd504c37799 100644
--- a/sys/netinet6/nd6.c
+++ b/sys/netinet6/nd6.c
@@ -138,7 +138,6 @@ static void nd6_free(struct llentry **, int);
static void nd6_free_redirect(const struct llentry *);
static void nd6_llinfo_timer(void *);
static void nd6_llinfo_settimer_locked(struct llentry *, long);
-static void clear_llinfo_pqueue(struct llentry *);
static int nd6_resolve_slow(struct ifnet *, int, int, struct mbuf *,
const struct sockaddr_in6 *, u_char *, uint32_t *, struct llentry **);
static int nd6_need_cache(struct ifnet *);
@@ -804,18 +803,19 @@ nd6_llinfo_timer(void *arg)
/* Send NS to multicast address */
pdst = NULL;
} else {
- struct mbuf *m = ln->la_hold;
- if (m) {
- struct mbuf *m0;
+ struct mbuf *m;
+ ICMP6STAT_ADD(icp6s_dropped, ln->la_numheld);
+
+ m = ln->la_hold;
+ if (m != NULL) {
/*
* assuming every packet in la_hold has the
* same IP header. Send error after unlock.
*/
- m0 = m->m_nextpkt;
+ ln->la_hold = m->m_nextpkt;
m->m_nextpkt = NULL;
- ln->la_hold = m0;
- clear_llinfo_pqueue(ln);
+ ln->la_numheld--;
}
nd6_free(&ln, 0);
if (m != NULL) {
@@ -2199,6 +2199,7 @@ nd6_grab_holdchain(struct llentry *ln)
chain = ln->la_hold;
ln->la_hold = NULL;
+ ln->la_numheld = 0;
if (ln->ln_state == ND6_LLINFO_STALE) {
/*
@@ -2418,6 +2419,7 @@ nd6_resolve_slow(struct ifnet *ifp, int family, int flags, struct mbuf *m,
struct in6_addr *psrc, src;
int send_ns, ll_len;
char *lladdr;
+ size_t dropped;
NET_EPOCH_ASSERT();
@@ -2484,28 +2486,8 @@ nd6_resolve_slow(struct ifnet *ifp, int family, int flags, struct mbuf *m,
* packet queue in the mbuf. When it exceeds nd6_maxqueuelen,
* the oldest packet in the queue will be removed.
*/
-
- if (lle->la_hold != NULL) {
- struct mbuf *m_hold;
- int i;
-
- i = 0;
- for (m_hold = lle->la_hold; m_hold; m_hold = m_hold->m_nextpkt){
- i++;
- if (m_hold->m_nextpkt == NULL) {
- m_hold->m_nextpkt = m;
- break;
- }
- }
- while (i >= V_nd6_maxqueuelen) {
- m_hold = lle->la_hold;
- lle->la_hold = lle->la_hold->m_nextpkt;
- m_freem(m_hold);
- i--;
- }
- } else {
- lle->la_hold = m;
- }
+ dropped = lltable_append_entry_queue(lle, m, V_nd6_maxqueuelen);
+ ICMP6STAT_ADD(icp6s_dropped, dropped);
/*
* If there has been no NS for the neighbor after entering the
@@ -2700,19 +2682,6 @@ nd6_rem_ifa_lle(struct in6_ifaddr *ia, int all)
lltable_delete_addr(LLTABLE6(ifp), LLE_IFADDR, saddr);
}
-static void
-clear_llinfo_pqueue(struct llentry *ln)
-{
- struct mbuf *m_hold, *m_hold_next;
-
- for (m_hold = ln->la_hold; m_hold; m_hold = m_hold_next) {
- m_hold_next = m_hold->m_nextpkt;
- m_freem(m_hold);
- }
-
- ln->la_hold = NULL;
-}
-
static int
nd6_sysctl_prlist(SYSCTL_HANDLER_ARGS)
{
diff --git a/usr.bin/netstat/inet6.c b/usr.bin/netstat/inet6.c
index b8d954dbd939..2724590de7d3 100644
--- a/usr.bin/netstat/inet6.c
+++ b/usr.bin/netstat/inet6.c
@@ -994,6 +994,8 @@ icmp6_stats(u_long off, const char *name, int af1 __unused, int proto __unused)
"{N:/bad checksum%s}\n");
p(icp6s_badlen, "\t{:dropped-bad-length/%ju} "
"{N:/message%s with bad length}\n");
+ p(icp6s_dropped, "{:dropped-no-entry/%ju} "
+ "{N:/total packet%s dropped due to failed NDP resolution}\n");
#define NELEM (int)(sizeof(icmp6stat.icp6s_inhist)/sizeof(icmp6stat.icp6s_inhist[0]))
for (first = 1, i = 0; i < NELEM; i++)
if (icmp6stat.icp6s_inhist[i] != 0) {