Date: Tue, 11 May 2021 10:37:31 GMT From: Wojciech Macek <wma@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 65634ae748e7 - main - mroute: fix race condition during mrouter shutting down Message-ID: <202105111037.14BAbVuU090518@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by wma: URL: https://cgit.FreeBSD.org/src/commit/?id=65634ae748e7f6b7b9f11e8838c65060c3f31347 commit 65634ae748e7f6b7b9f11e8838c65060c3f31347 Author: Wojciech Macek <wma@FreeBSD.org> AuthorDate: 2021-04-23 03:57:03 +0000 Commit: Wojciech Macek <wma@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(); } /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202105111037.14BAbVuU090518>