From nobody Mon Feb 20 15:25:45 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4PL5pd6fzhz3sxdF; Mon, 20 Feb 2023 15:25:45 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4PL5pd697kz429h; Mon, 20 Feb 2023 15:25:45 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1676906745; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=2Izo2YSnHcd2MFpgJhq87ry0GqUjIBMOBEeG9/cObGI=; b=W2fpkviL4YngQpbFO+AdR+aWpAUmdGkMvcfUngQIrGyyINfM+jF3Tevk4o1mP/TS/o4jjN Fs9e1Go/29NZOCFhPkh2MlhT31U4VF02HBGmtJ41QyMzwvU+TnO3SrwpuQEsCwZGMQTqZs ytQmnQE8wuSzOTKhU02hv2uYcZoes3/Wxlj8N+o//bmtRfpSA0MuMmmGjSVRLtd1Y3OTT7 QWuriob6wMziVfRH5b7tWl/AdvHSWClqVcX+jvkH5cFOYGu6UI5/kfkiG7n8thjeDl6uLA /OI6i4aZ9kEU+NBBZl0/CL8nE3KnjldSLNv74UpXtXN4yv+qEa0rqQ/DL/ISzw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1676906745; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=2Izo2YSnHcd2MFpgJhq87ry0GqUjIBMOBEeG9/cObGI=; b=amYwCAP+vhYIQ7jnS91zaWJQNXrbWDKJZQOgjYMJjBczLpwKyn9vY5jZuGMir1qGfeaUwX ELPyATg1VmFNdSOI/tP9V8C/qSWKfPp8wlzRus6DZPooNGsvEs+/0Brv6aRnb8Y3So8CGD Za1bzCTFI0mvm0KpWXRP7apVSFauJnyu1I6oolyhJaxkeeVmbzZbzvpNTK0tGH0gq0vnyr wamFeZzZRCfhU+hL2/nvgz4HKfSLb1o0zTLNvGiYSN2neMH+XGX/m9J71vo1Gf9n+sf6lj RRCgzlE0nYtQN791JRIvDd7stib1DlhVF/TcSHSln9wj8170DzUb4kYWEYnVRQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1676906745; a=rsa-sha256; cv=none; b=WflG+uBVlvZyPb2t9q7Mj5tmD6dOKEUKw3V+g3nEesQ49XyDCNmQ+Ynx+k6t9rRsYRzJSA UuDyWpRjzYGHHHnAnsKOndYCcddUSxJnzBuDTGUtC95PjaqaFiwGFWlX2moPWP1Nhrj6+A VR7Rw2BqAakyye/j9msvr8HFmPbvRFLtTJqXaQAW13a1ZQi2Pyx7zUg0mf9qw+rYicmFwn WeFb5bcNFeCLAHYlZfmzHGoWh3I2ggwMmDktJLxNLc4+iZfJiWYbGeA7WEQH7DkPH2Pgx0 84y6tIx0fONxJnswt4Pn3hHX+ySVM4uuHDwUt/HBvI5+McoVNFd3AAfGgsiohg== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4PL5pd5GWVzmL2; Mon, 20 Feb 2023 15:25:45 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 31KFPjgJ078273; Mon, 20 Feb 2023 15:25:45 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 31KFPj9X078272; Mon, 20 Feb 2023 15:25:45 GMT (envelope-from git) Date: Mon, 20 Feb 2023 15:25:45 GMT Message-Id: <202302201525.31KFPj9X078272@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: "Alexander V. Chernikov" Subject: git: a38e2ff92458 - stable/13 - netinet6: Fix mbuf leak in NDP List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: melifaro X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: a38e2ff92458b97db06fccf9036c6f74d80155a2 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by melifaro: URL: https://cgit.FreeBSD.org/src/commit/?id=a38e2ff92458b97db06fccf9036c6f74d80155a2 commit a38e2ff92458b97db06fccf9036c6f74d80155a2 Author: Arseny Smalyuk AuthorDate: 2022-05-31 20:04:51 +0000 Commit: Alexander V. Chernikov CommitDate: 2023-02-20 15:14:24 +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 (cherry picked from commit d18b4bec98f1cf3c51103a22c0c041e6238c44c7) --- 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 05cd8ea24a46..9ada3af318f3 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 143b000adc22..6af13660e344 100644 --- a/sys/net/if_llatbl.h +++ b/sys/net/if_llatbl.h @@ -221,6 +221,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 59a7c78cb03b..d252d8be9a99 100644 --- a/sys/netinet/if_ether.c +++ b/sys/netinet/if_ether.c @@ -465,8 +465,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; @@ -534,6 +532,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 @@ -541,24 +540,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 924727d598de..be881b6291ac 100644 --- a/sys/netinet6/nd6.c +++ b/sys/netinet6/nd6.c @@ -139,7 +139,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 *); @@ -805,18 +804,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) { @@ -2149,6 +2149,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) { /* @@ -2368,6 +2369,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(); @@ -2434,28 +2436,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 @@ -2650,19 +2632,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) {