From owner-dev-commits-src-main@freebsd.org Tue May 11 10:37:32 2021 Return-Path: Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 229896323C9; Tue, 11 May 2021 10:37:32 +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 4FfZB40VGsz3kVZ; Tue, 11 May 2021 10:37:32 +0000 (UTC) (envelope-from git@FreeBSD.org) 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 040E5167D3; Tue, 11 May 2021 10:37:32 +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 14BAbVeq090519; Tue, 11 May 2021 10:37:31 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 14BAbVuU090518; Tue, 11 May 2021 10:37:31 GMT (envelope-from git) Date: Tue, 11 May 2021 10:37:31 GMT Message-Id: <202105111037.14BAbVuU090518@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Wojciech Macek Subject: git: 65634ae748e7 - main - mroute: fix race condition during mrouter shutting down MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: wma X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 65634ae748e7f6b7b9f11e8838c65060c3f31347 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 May 2021 10:37:32 -0000 The branch main has been updated by wma: URL: https://cgit.FreeBSD.org/src/commit/?id=65634ae748e7f6b7b9f11e8838c65060c3f31347 commit 65634ae748e7f6b7b9f11e8838c65060c3f31347 Author: Wojciech Macek AuthorDate: 2021-04-23 03:57:03 +0000 Commit: Wojciech Macek CommitDate: 2021-05-11 10:34:20 +0000 mroute: fix race condition during mrouter shutting down There is a race condition between V_ip_mrouter de-init and ip_mforward handling. It might happen that mrouted is cleaned up after V_ip_mrouter check and before processing packet in ip_mforward. Use epoch call aproach, similar to IPSec which also handles such case. Reported by: Damien Deville Obtained from: Stormshield Reviewed by: mw Differential Revision: https://reviews.freebsd.org/D29946 --- sys/netinet/ip_input.c | 9 ++++++++- sys/netinet/ip_mroute.c | 2 ++ sys/netinet/ip_mroute.h | 5 +++++ sys/netinet/ip_output.c | 5 +++++ 4 files changed, 20 insertions(+), 1 deletion(-) diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c index a85f8ac7b567..43d375c2385f 100644 --- a/sys/netinet/ip_input.c +++ b/sys/netinet/ip_input.c @@ -82,6 +82,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include @@ -451,6 +452,7 @@ ip_direct_input(struct mbuf *m) void ip_input(struct mbuf *m) { + MROUTER_RLOCK_TRACKER; struct rm_priotracker in_ifa_tracker; struct ip *ip = NULL; struct in_ifaddr *ia = NULL; @@ -743,6 +745,7 @@ passin: return; } if (IN_MULTICAST(ntohl(ip->ip_dst.s_addr))) { + MROUTER_RLOCK(); if (V_ip_mrouter) { /* * If we are acting as a multicast router, all @@ -753,6 +756,7 @@ passin: * must be discarded, else it may be accepted below. */ if (ip_mforward && ip_mforward(ip, ifp, m, 0) != 0) { + MROUTER_RUNLOCK(); IPSTAT_INC(ips_cantforward); m_freem(m); return; @@ -763,10 +767,13 @@ passin: * all multicast IGMP packets, whether or not this * host belongs to their destination groups. */ - if (ip->ip_p == IPPROTO_IGMP) + if (ip->ip_p == IPPROTO_IGMP) { + MROUTER_RUNLOCK(); goto ours; + } IPSTAT_INC(ips_forward); } + MROUTER_RUNLOCK(); /* * Assume the packet is for us, to avoid prematurely taking * a lock on the in_multi hash. Protocols must perform diff --git a/sys/netinet/ip_mroute.c b/sys/netinet/ip_mroute.c index b66fe8df0793..8ecc983ea58f 100644 --- a/sys/netinet/ip_mroute.c +++ b/sys/netinet/ip_mroute.c @@ -720,6 +720,8 @@ X_ip_mrouter_done(void) ip_mrouter_cnt--; V_mrt_api_config = 0; + MROUTER_WAIT(); + VIF_LOCK(); /* diff --git a/sys/netinet/ip_mroute.h b/sys/netinet/ip_mroute.h index 054eb921d3df..6ef99c0172f3 100644 --- a/sys/netinet/ip_mroute.h +++ b/sys/netinet/ip_mroute.h @@ -355,6 +355,11 @@ extern int (*ip_mrouter_get)(struct socket *, struct sockopt *); extern int (*ip_mrouter_done)(void); extern int (*mrt_ioctl)(u_long, caddr_t, int); +#define MROUTER_RLOCK_TRACKER struct epoch_tracker mrouter_et +#define MROUTER_RLOCK() epoch_enter_preempt(net_epoch_preempt, &mrouter_et) +#define MROUTER_RUNLOCK() epoch_exit_preempt(net_epoch_preempt, &mrouter_et) +#define MROUTER_WAIT() epoch_wait_preempt(net_epoch_preempt) + #endif /* _KERNEL */ #endif /* _NETINET_IP_MROUTE_H_ */ diff --git a/sys/netinet/ip_output.c b/sys/netinet/ip_output.c index 405490e890c0..f913a2591fcc 100644 --- a/sys/netinet/ip_output.c +++ b/sys/netinet/ip_output.c @@ -83,6 +83,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include @@ -319,6 +320,7 @@ int ip_output(struct mbuf *m, struct mbuf *opt, struct route *ro, int flags, struct ip_moptions *imo, struct inpcb *inp) { + MROUTER_RLOCK_TRACKER; struct rm_priotracker in_ifa_tracker; struct ip *ip; struct ifnet *ifp = NULL; /* keep compiler happy */ @@ -613,6 +615,7 @@ again: * above, will be forwarded by the ip_input() routine, * if necessary. */ + MROUTER_RLOCK(); if (V_ip_mrouter && (flags & IP_FORWARDING) == 0) { /* * If rsvp daemon is not running, do not @@ -624,10 +627,12 @@ again: imo = NULL; if (ip_mforward && ip_mforward(ip, ifp, m, imo) != 0) { + MROUTER_RUNLOCK(); m_freem(m); goto done; } } + MROUTER_RUNLOCK(); } /*