git: 65634ae748e7 - main - mroute: fix race condition during mrouter shutting down

Wojciech Macek wma at FreeBSD.org
Tue May 11 10:37:32 UTC 2021


The branch main has been updated by wma:

URL: https://cgit.FreeBSD.org/src/commit/?id=65634ae748e7f6b7b9f11e8838c65060c3f31347

commit 65634ae748e7f6b7b9f11e8838c65060c3f31347
Author:     Wojciech Macek <wma at FreeBSD.org>
AuthorDate: 2021-04-23 03:57:03 +0000
Commit:     Wojciech Macek <wma at FreeBSD.org>
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 <machine/in_cksum.h>
 #include <netinet/ip_carp.h>
 #include <netinet/in_rss.h>
+#include <netinet/ip_mroute.h>
 
 #include <netipsec/ipsec_support.h>
 
@@ -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 <netinet/in_var.h>
 #include <netinet/ip_var.h>
 #include <netinet/ip_options.h>
+#include <netinet/ip_mroute.h>
 
 #include <netinet/udp.h>
 #include <netinet/udp_var.h>
@@ -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();
 		}
 
 		/*


More information about the dev-commits-src-main mailing list