git: a0d2e5ebaa2e - main - if_gre(4): Fix races by changing initialization order and locks
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 30 Jul 2026 18:11:52 UTC
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.hdr