Date: Thu, 30 Jul 2026 18:11:52 +0000 From: Pouria Mousavizadeh Tehrani <pouria@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: a0d2e5ebaa2e - main - if_gre(4): Fix races by changing initialization order and locks Message-ID: <6a6b93e8.3ddca.39012bf@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by pouria: URL: https://cgit.FreeBSD.org/src/commit/?id=a0d2e5ebaa2e7a8da83f593ed5459926423afa20 commit a0d2e5ebaa2e7a8da83f593ed5459926423afa20 Author: Pouria Mousavizadeh Tehrani <pouria@FreeBSD.org> AuthorDate: 2026-07-30 17:48:47 +0000 Commit: Pouria Mousavizadeh Tehrani <pouria@FreeBSD.org> CommitDate: 2026-07-30 17:51:11 +0000 if_gre(4): Fix races by changing initialization order and locks Treat if_gre like any other network drivers during module initialization by using SI_SUB_PROTO_IF. Also, destroy cloned interfaces via a prison removal callback for gre over udp. PR: 275474 Reviewed by: markj Discussed with: glebius Differential Revision: https://reviews.freebsd.org/D57669 --- sys/net/if_gre.c | 96 ++++++++++++++++++++++++++++++++++++-------------------- sys/net/if_gre.h | 3 -- 2 files changed, 62 insertions(+), 37 deletions(-) diff --git a/sys/net/if_gre.c b/sys/net/if_gre.c index 563478eb67f8..1dc82238c6ab 100644 --- a/sys/net/if_gre.c +++ b/sys/net/if_gre.c @@ -40,11 +40,13 @@ #include "opt_rss.h" #include <sys/param.h> +#include <sys/jail.h> #include <sys/kernel.h> #include <sys/lock.h> #include <sys/malloc.h> #include <sys/module.h> #include <sys/mbuf.h> +#include <sys/osd.h> #include <sys/priv.h> #include <sys/proc.h> #include <sys/socket.h> @@ -208,14 +210,16 @@ vnet_gre_init(const void *unused __unused) in6_gre_init(); #endif } -VNET_SYSINIT(vnet_gre_init, SI_SUB_PROTO_IFATTACHDOMAIN, SI_ORDER_ANY, - vnet_gre_init, NULL); +VNET_SYSINIT(vnet_gre_init, SI_SUB_PROTO_IF, SI_ORDER_ANY, vnet_gre_init, NULL); static void vnet_gre_uninit(const void *unused __unused) { - ifc_detach_cloner(V_gre_cloner); + if (V_gre_cloner != NULL) { + ifc_detach_cloner(V_gre_cloner); + V_gre_cloner = NULL; + } #ifdef INET in_gre_uninit(); #endif @@ -224,8 +228,8 @@ vnet_gre_uninit(const void *unused __unused) #endif /* XXX: epoch_call drain */ } -VNET_SYSUNINIT(vnet_gre_uninit, SI_SUB_PROTO_IFATTACHDOMAIN, SI_ORDER_ANY, - vnet_gre_uninit, NULL); +VNET_SYSUNINIT(vnet_gre_uninit, SI_SUB_PROTO_IF, SI_ORDER_ANY, vnet_gre_uninit, + NULL); static int gre_clone_create_nl(struct if_clone *ifc, char *name, size_t len, @@ -248,7 +252,7 @@ gre_clone_modify_nl(struct ifnet *ifp, struct ifc_data_nl *ifd) struct nl_parsed_gre params; struct nlattr *attrs = lattrs->ifla_idata; struct nlattr_bmask bm; - int error = 0; + int error; if ((attrs == NULL) || (nl_has_attr(ifd->bm, IFLA_LINKINFO) == 0)) { @@ -267,6 +271,11 @@ gre_clone_modify_nl(struct ifnet *ifp, struct ifc_data_nl *ifd) if ((error = nl_parse_nested(attrs, &gre_modify_parser, npt, ¶ms)) != 0) return (error); + sx_xlock(&gre_ioctl_sx); + sc = ifp->if_softc; + if (sc == NULL) + goto generic; + if (nl_has_attr(&bm, IFLA_GRE_LOCAL) && nl_has_attr(&bm, IFLA_GRE_REMOTE)) error = gre_set_addr_nl(sc, npt, params.ifla_local, params.ifla_remote); else if (nl_has_attr(&bm, IFLA_GRE_LOCAL) || nl_has_attr(&bm, IFLA_GRE_REMOTE)) { @@ -286,6 +295,9 @@ gre_clone_modify_nl(struct ifnet *ifp, struct ifc_data_nl *ifd) if (error == 0 && nl_has_attr(&bm, IFLA_GRE_ENCAP_SPORT)) error = gre_set_udp_sport_nl(sc, npt, params.ifla_encap_sport); +generic: + sx_xunlock(&gre_ioctl_sx); + if (error == 0) error = nl_modify_ifp_generic(ifp, ifd->lattrs, ifd->bm, ifd->npt); @@ -295,7 +307,6 @@ gre_clone_modify_nl(struct ifnet *ifp, struct ifc_data_nl *ifd) static void gre_clone_dump_nl(struct ifnet *ifp, struct nl_writer *nw) { - GRE_RLOCK_TRACKER; struct gre_softc *sc; nlattr_add_u32(nw, IFLA_LINK, ifp->if_index); @@ -312,8 +323,10 @@ gre_clone_dump_nl(struct ifnet *ifp, struct nl_writer *nw) return; } + sx_slock(&gre_ioctl_sx); sc = ifp->if_softc; - GRE_RLOCK(); + if (sc == NULL) + goto ret; if (sc->gre_family == AF_INET) { #ifdef INET @@ -343,10 +356,11 @@ gre_clone_dump_nl(struct ifnet *ifp, struct nl_writer *nw) sc->gre_options & GRE_UDPENCAP ? IFLA_TUNNEL_GRE_UDP : IFLA_TUNNEL_NONE); nlattr_add_u16(nw, IFLA_GRE_ENCAP_SPORT, sc->gre_port); +ret: nlattr_set_len(nw, off2); nlattr_set_len(nw, off); - GRE_RUNLOCK(); + sx_sunlock(&gre_ioctl_sx); } static int @@ -391,6 +405,7 @@ gre_reassign(struct ifnet *ifp, struct vnet *new_vnet __unused, if (sc != NULL) gre_delete_tunnel(sc); sx_xunlock(&gre_ioctl_sx); + if_link_state_change(ifp, LINK_STATE_DOWN); } #endif /* VIMAGE */ @@ -402,10 +417,11 @@ gre_clone_destroy(struct if_clone *ifc, struct ifnet *ifp, uint32_t flags) sx_xlock(&gre_ioctl_sx); sc = ifp->if_softc; gre_delete_tunnel(sc); - bpfdetach(ifp); - if_detach(ifp); ifp->if_softc = NULL; sx_xunlock(&gre_ioctl_sx); + if_link_state_change(GRE2IFP(sc), LINK_STATE_DOWN); + bpfdetach(ifp); + if_detach(ifp); GRE_WAIT(); if_free(ifp); @@ -627,7 +643,7 @@ gre_delete_tunnel(struct gre_softc *sc) { struct gre_socket *gs; - sx_assert(&gre_ioctl_sx, SA_XLOCKED); + GRE_LOCK_ASSERT(); if (sc->gre_family != 0) { CK_LIST_REMOVE(sc, chain); CK_LIST_REMOVE(sc, srchash); @@ -646,7 +662,6 @@ gre_delete_tunnel(struct gre_softc *sc) sc->gre_so = NULL; } GRE2IFP(sc)->if_drv_flags &= ~IFF_DRV_RUNNING; - if_link_state_change(GRE2IFP(sc), LINK_STATE_DOWN); } struct gre_list * @@ -911,7 +926,6 @@ gre_flowid(struct gre_softc *sc, struct mbuf *m, uint32_t af) static int gre_transmit(struct ifnet *ifp, struct mbuf *m) { - GRE_RLOCK_TRACKER; struct gre_softc *sc; struct grehdr *gh; struct udphdr *uh; @@ -919,8 +933,8 @@ gre_transmit(struct ifnet *ifp, struct mbuf *m) int error, len; uint16_t proto; + NET_EPOCH_ASSERT(); len = 0; - GRE_RLOCK(); #ifdef MAC error = mac_ifnet_check_transmit(ifp, m); if (error) { @@ -1028,7 +1042,6 @@ drop: if_inc_counter(ifp, IFCOUNTER_OPACKETS, 1); if_inc_counter(ifp, IFCOUNTER_OBYTES, len); } - GRE_RUNLOCK(); return (error); } @@ -1061,18 +1074,14 @@ gre_set_addr_nl(struct gre_softc *sc, struct nl_pstate *npt, else if (src->sa_family == AF_INET) { memcpy(&aliasreq.in.ifra_addr, src, sizeof(struct sockaddr_in)); memcpy(&aliasreq.in.ifra_dstaddr, dst, sizeof(struct sockaddr_in)); - sx_xlock(&gre_ioctl_sx); error = in_gre_ioctl(sc, SIOCSIFPHYADDR, (caddr_t)&aliasreq.in); - sx_xunlock(&gre_ioctl_sx); } #endif #ifdef INET6 else if (src->sa_family == AF_INET6) { memcpy(&aliasreq.in6.ifra_addr, src, sizeof(struct sockaddr_in6)); memcpy(&aliasreq.in6.ifra_dstaddr, dst, sizeof(struct sockaddr_in6)); - sx_xlock(&gre_ioctl_sx); error = in6_gre_ioctl(sc, SIOCSIFPHYADDR_IN6, (caddr_t)&aliasreq.in6); - sx_xunlock(&gre_ioctl_sx); } #endif else @@ -1091,11 +1100,9 @@ gre_set_addr_nl(struct gre_softc *sc, struct nl_pstate *npt, static int gre_set_flags_nl(struct gre_softc *sc, struct nl_pstate *npt, uint32_t opt) { - int error = 0; + int error; - sx_xlock(&gre_ioctl_sx); error = gre_set_flags(sc, opt); - sx_xunlock(&gre_ioctl_sx); if (error == EINVAL) nlmsg_report_err_msg(npt, "gre flags are invalid"); @@ -1106,11 +1113,9 @@ gre_set_flags_nl(struct gre_softc *sc, struct nl_pstate *npt, uint32_t opt) static int gre_set_key_nl(struct gre_softc *sc, struct nl_pstate *npt, uint32_t key) { - int error = 0; + int error; - sx_xlock(&gre_ioctl_sx); error = gre_set_key(sc, key); - sx_xunlock(&gre_ioctl_sx); if (error == EINVAL) nlmsg_report_err_msg(npt, "gre key is invalid: %u", key); @@ -1122,16 +1127,15 @@ static int gre_set_encap_nl(struct gre_softc *sc, struct nl_pstate *npt, uint32_t type) { uint32_t opt; - int error = 0; + int error; - sx_xlock(&gre_ioctl_sx); opt = sc->gre_options; if (type & IFLA_TUNNEL_GRE_UDP) opt |= GRE_UDPENCAP; else opt &= ~GRE_UDPENCAP; + error = gre_set_flags(sc, opt); - sx_xunlock(&gre_ioctl_sx); if (error == EEXIST) nlmsg_report_err_msg(npt, "same gre tunnel exist"); @@ -1143,11 +1147,9 @@ gre_set_encap_nl(struct gre_softc *sc, struct nl_pstate *npt, uint32_t type) static int gre_set_udp_sport_nl(struct gre_softc *sc, struct nl_pstate *npt, uint16_t port) { - int error = 0; + int error; - sx_xlock(&gre_ioctl_sx); error = gre_set_udp_sport(sc, port); - sx_xunlock(&gre_ioctl_sx); if (error == EINVAL) nlmsg_report_err_msg(npt, "source port is invalid: %u", port); @@ -1156,15 +1158,41 @@ gre_set_udp_sport_nl(struct gre_softc *sc, struct nl_pstate *npt, uint16_t port) } +static int +gre_prison_remove(void *obj, void *data __unused) +{ +#ifdef VIMAGE + struct prison *pr; + + pr = obj; + if (prison_owns_vnet(pr)) { + CURVNET_SET(pr->pr_vnet); + if (V_gre_cloner != NULL) + vnet_gre_uninit(NULL); + CURVNET_RESTORE(); + } +#endif + return (0); +} + + static int gremodevent(module_t mod, int type, void *data) { + static int gre_osd_jail_slot; switch (type) { - case MOD_LOAD: + case MOD_LOAD: { + osd_method_t methods[PR_MAXMETHOD] = { + [PR_METHOD_REMOVE] = gre_prison_remove, + }; + gre_osd_jail_slot = osd_jail_register(NULL, methods); NL_VERIFY_PARSERS(all_parsers); break; + } case MOD_UNLOAD: + if (gre_osd_jail_slot != 0) + osd_jail_deregister(gre_osd_jail_slot); break; default: return (EOPNOTSUPP); @@ -1178,5 +1206,5 @@ static moduledata_t gre_mod = { 0 }; -DECLARE_MODULE(if_gre, gre_mod, SI_SUB_PSEUDO, SI_ORDER_ANY); +DECLARE_MODULE(if_gre, gre_mod, SI_SUB_PROTO_IF, SI_ORDER_ANY); MODULE_VERSION(if_gre, 1); diff --git a/sys/net/if_gre.h b/sys/net/if_gre.h index ae6d18385dfb..61c8ef54523e 100644 --- a/sys/net/if_gre.h +++ b/sys/net/if_gre.h @@ -117,9 +117,6 @@ MALLOC_DECLARE(M_GRE); #endif #define GRE2IFP(sc) ((sc)->gre_ifp) -#define GRE_RLOCK_TRACKER struct epoch_tracker gre_et -#define GRE_RLOCK() epoch_enter_preempt(net_epoch_preempt, &gre_et) -#define GRE_RUNLOCK() epoch_exit_preempt(net_epoch_preempt, &gre_et) #define GRE_WAIT() epoch_wait_preempt(net_epoch_preempt) #define gre_hdr gre_uhdr.hdrhome | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a6b93e8.3ddca.39012bf>