This diff converts a struct ifnet pointer in pfsync's softc into an
ifindex with corresponding if_get()/if_put() calls.
Seems to still work fine and fixes the following panic reported by double-p:
ifconfig pfsync0 syncdev vlan5 up
ifconfig vlan5 destroy
ifconfig pfsync0 destroy
-> crash because of stale pointer sc->sc_sync_if
ddb> trace
hook_disestablish() at hook_disestablish+0x25
pfsync_clone_destroy() at pfsync_clone_destroy+0x85
ifioctl() at ifioctl+0x232
sys_ioctl() at sys_ioctl+0x196
syscall() at syscall+0x19b
--- syscall (number 54) ---
end of kernel
end trace frame
Index: if_pfsync.c
===================================================================
RCS file: /cvs/src/sys/net/if_pfsync.c,v
retrieving revision 1.245
diff -u -p -r1.245 if_pfsync.c
--- if_pfsync.c 20 Feb 2017 06:30:39 -0000 1.245
+++ if_pfsync.c 9 Mar 2017 06:55:28 -0000
@@ -196,7 +196,7 @@ void pfsync_out_tdb(struct tdb *, void *
struct pfsync_softc {
struct ifnet sc_if;
- struct ifnet *sc_sync_if;
+ int sc_syncdev_ifidx;
struct pool sc_pool;
@@ -355,6 +355,7 @@ pfsync_clone_destroy(struct ifnet *ifp)
{
struct pfsync_softc *sc = ifp->if_softc;
struct pfsync_deferral *pd;
+ struct ifnet *syncdev;
timeout_del(&sc->sc_bulkfail_tmo);
timeout_del(&sc->sc_bulk_tmo);
@@ -365,10 +366,13 @@ pfsync_clone_destroy(struct ifnet *ifp)
if (sc->sc_link_demoted)
carp_group_demote_adj(&sc->sc_if, -1, "pfsync destroy");
#endif
- if (sc->sc_sync_if)
+ syncdev = if_get(sc->sc_syncdev_ifidx);
+ if (syncdev) {
hook_disestablish(
- sc->sc_sync_if->if_linkstatehooks,
+ syncdev->if_linkstatehooks,
sc->sc_lhcookie);
+ if_put(syncdev);
+ }
if_detach(ifp);
pfsync_drop(sc);
@@ -401,11 +405,15 @@ void
pfsync_syncdev_state(void *arg)
{
struct pfsync_softc *sc = arg;
+ struct ifnet *syncdev;
- if (!sc->sc_sync_if || !(sc->sc_if.if_flags & IFF_UP))
+ syncdev = if_get(sc->sc_syncdev_ifidx);
+ if (!syncdev || !(sc->sc_if.if_flags & IFF_UP)) {
+ if_put(syncdev);
return;
+ }
- if (sc->sc_sync_if->if_link_state == LINK_STATE_DOWN) {
+ if (syncdev->if_link_state == LINK_STATE_DOWN) {
sc->sc_if.if_flags &= ~IFF_RUNNING;
if (!sc->sc_link_demoted) {
#if NCARP > 0
@@ -425,6 +433,8 @@ pfsync_syncdev_state(void *arg)
pfsync_request_full_update(sc);
}
+
+ if_put(syncdev);
}
int
@@ -640,6 +650,7 @@ pfsync_input(struct mbuf **mp, int *offp
{
struct mbuf *n, *m = *mp;
struct pfsync_softc *sc = pfsyncif;
+ struct ifnet *syncdev;
struct ip *ip = mtod(m, struct ip *);
struct pfsync_header *ph;
struct pfsync_subheader subh;
@@ -649,11 +660,14 @@ pfsync_input(struct mbuf **mp, int *offp
/* verify that we have a sync interface configured */
if (sc == NULL || !ISSET(sc->sc_if.if_flags, IFF_RUNNING) ||
- sc->sc_sync_if == NULL || !pf_status.running)
+ !pf_status.running)
+ goto done;
+ syncdev = if_get(sc->sc_syncdev_ifidx);
+ if (syncdev == NULL)
goto done;
/* verify that the packet came in on the right interface */
- if (sc->sc_sync_if->if_index != m->m_pkthdr.ph_ifidx) {
+ if (sc->sc_syncdev_ifidx != m->m_pkthdr.ph_ifidx) {
pfsyncstat_inc(pfsyncs_badif);
goto done;
}
@@ -729,6 +743,7 @@ pfsync_input(struct mbuf **mp, int *offp
done:
m_freem(m);
+ if_put(syncdev);
return IPPROTO_DONE;
}
@@ -1234,7 +1249,7 @@ pfsyncioctl(struct ifnet *ifp, u_long cm
struct ifreq *ifr = (struct ifreq *)data;
struct ip_moptions *imo = &sc->sc_imo;
struct pfsyncreq pfsyncr;
- struct ifnet *sifp;
+ struct ifnet *sifp, *syncdev = NULL;
struct ip *ip;
int s, error;
@@ -1269,10 +1284,13 @@ pfsyncioctl(struct ifnet *ifp, u_long cm
splx(s);
break;
case SIOCSIFMTU:
- if (!sc->sc_sync_if ||
+ syncdev = if_get(sc->sc_syncdev_ifidx);
+ if (!syncdev ||
ifr->ifr_mtu <= PFSYNC_MINPKT ||
- ifr->ifr_mtu > sc->sc_sync_if->if_mtu)
+ ifr->ifr_mtu > syncdev->if_mtu) {
+ if_put(syncdev);
return (EINVAL);
+ }
s = splnet();
if (ifr->ifr_mtu < ifp->if_mtu)
pfsync_sendout();
@@ -1281,9 +1299,11 @@ pfsyncioctl(struct ifnet *ifp, u_long cm
break;
case SIOCGETPFSYNC:
bzero(&pfsyncr, sizeof(pfsyncr));
- if (sc->sc_sync_if) {
- strlcpy(pfsyncr.pfsyncr_syncdev,
- sc->sc_sync_if->if_xname, IFNAMSIZ);
+ syncdev = if_get(sc->sc_syncdev_ifidx);
+ if (syncdev) {
+ strlcpy(pfsyncr.pfsyncr_syncdev, syncdev->if_xname,
+ IFNAMSIZ);
+ if_put(syncdev);
}
pfsyncr.pfsyncr_syncpeer = sc->sc_sync_peer;
pfsyncr.pfsyncr_maxupdates = sc->sc_maxupdates;
@@ -1313,11 +1333,15 @@ pfsyncioctl(struct ifnet *ifp, u_long cm
sc->sc_defer = pfsyncr.pfsyncr_defer;
if (pfsyncr.pfsyncr_syncdev[0] == 0) {
- if (sc->sc_sync_if)
+ syncdev = if_get(sc->sc_syncdev_ifidx);
+ if (syncdev) {
hook_disestablish(
- sc->sc_sync_if->if_linkstatehooks,
+ syncdev->if_linkstatehooks,
sc->sc_lhcookie);
- sc->sc_sync_if = NULL;
+ if_put(syncdev);
+ }
+ sc->sc_syncdev_ifidx = 0;
+ syncdev = NULL;
if (imo->imo_num_memberships > 0) {
in_delmulti(imo->imo_membership[
--imo->imo_num_memberships]);
@@ -1332,29 +1356,33 @@ pfsyncioctl(struct ifnet *ifp, u_long cm
return (EINVAL);
}
+ syncdev = if_get(sc->sc_syncdev_ifidx);
if (sifp->if_mtu < sc->sc_if.if_mtu ||
- (sc->sc_sync_if != NULL &&
- sifp->if_mtu < sc->sc_sync_if->if_mtu) ||
+ (syncdev != NULL &&
+ sifp->if_mtu < syncdev->if_mtu) ||
sifp->if_mtu < MCLBYTES - sizeof(struct ip))
pfsync_sendout();
- if (sc->sc_sync_if)
+ if (syncdev) {
hook_disestablish(
- sc->sc_sync_if->if_linkstatehooks,
+ syncdev->if_linkstatehooks,
sc->sc_lhcookie);
- sc->sc_sync_if = sifp;
+ if_put(syncdev);
+ }
+ syncdev = sifp;
+ sc->sc_syncdev_ifidx = syncdev->if_index;
if (imo->imo_num_memberships > 0) {
in_delmulti(imo->imo_membership[--imo->imo_num_memberships]);
imo->imo_ifidx = 0;
}
- if (sc->sc_sync_if &&
+ if (syncdev &&
sc->sc_sync_peer.s_addr == INADDR_PFSYNC_GROUP) {
struct in_addr addr;
- if (!(sc->sc_sync_if->if_flags & IFF_MULTICAST)) {
- sc->sc_sync_if = NULL;
+ if (!(syncdev->if_flags & IFF_MULTICAST)) {
+ sc->sc_syncdev_ifidx = 0;
splx(s);
return (EADDRNOTAVAIL);
}
@@ -1362,13 +1390,13 @@ pfsyncioctl(struct ifnet *ifp, u_long cm
addr.s_addr = INADDR_PFSYNC_GROUP;
if ((imo->imo_membership[0] =
- in_addmulti(&addr, sc->sc_sync_if)) == NULL) {
- sc->sc_sync_if = NULL;
+ in_addmulti(&addr, syncdev)) == NULL) {
+ sc->sc_syncdev_ifidx = 0;
splx(s);
return (ENOBUFS);
}
imo->imo_num_memberships++;
- imo->imo_ifidx = sc->sc_sync_if->if_index;
+ imo->imo_ifidx = syncdev->if_index;
imo->imo_ttl = PFSYNC_DFLTTL;
imo->imo_loop = 0;
}
@@ -1386,7 +1414,7 @@ pfsyncioctl(struct ifnet *ifp, u_long cm
ip->ip_dst.s_addr = sc->sc_sync_peer.s_addr;
sc->sc_lhcookie =
- hook_establish(sc->sc_sync_if->if_linkstatehooks, 1,
+ hook_establish(syncdev->if_linkstatehooks, 1,
pfsync_syncdev_state, sc);
pfsync_request_full_update(sc);
@@ -1487,6 +1515,7 @@ pfsync_sendout(void)
#if NBPFILTER > 0
struct ifnet *ifp = &sc->sc_if;
#endif
+ struct ifnet *syncdev;
struct mbuf *m;
struct ip *ip;
struct pfsync_header *ph;
@@ -1501,12 +1530,14 @@ pfsync_sendout(void)
if (sc == NULL || sc->sc_len == PFSYNC_MINPKT)
return;
+ syncdev = if_get(sc->sc_syncdev_ifidx);
if (!ISSET(sc->sc_if.if_flags, IFF_RUNNING) ||
#if NBPFILTER > 0
- (ifp->if_bpf == NULL && sc->sc_sync_if == NULL)) {
+ (ifp->if_bpf == NULL && syncdev == NULL)) {
#else
- sc->sc_sync_if == NULL) {
+ syncdev == NULL) {
#endif
+ if_put(syncdev);
pfsync_drop(sc);
return;
}
@@ -1515,6 +1546,7 @@ pfsync_sendout(void)
if (m == NULL) {
sc->sc_if.if_oerrors++;
pfsyncstat_inc(pfsyncs_onomem);
+ if_put(syncdev);
pfsync_drop(sc);
return;
}
@@ -1525,6 +1557,7 @@ pfsync_sendout(void)
m_free(m);
sc->sc_if.if_oerrors++;
pfsyncstat_inc(pfsyncs_onomem);
+ if_put(syncdev);
pfsync_drop(sc);
return;
}
@@ -1637,7 +1670,7 @@ pfsync_sendout(void)
m->m_len = m->m_pkthdr.len = sc->sc_len;
}
- if (sc->sc_sync_if == NULL) {
+ if (syncdev == NULL) {
sc->sc_len = PFSYNC_MINPKT;
m_freem(m);
return;
@@ -1656,6 +1689,8 @@ pfsync_sendout(void)
pfsyncstat_inc(pfsyncs_opackets);
else
pfsyncstat_inc(pfsyncs_oerrors);
+
+ if_put(syncdev);
}
void
@@ -1891,7 +1926,8 @@ pfsync_cancel_full_update(struct pfsync_
void
pfsync_request_full_update(struct pfsync_softc *sc)
{
- if (sc->sc_sync_if && ISSET(sc->sc_if.if_flags, IFF_RUNNING)) {
+ struct ifnet *syncdev = if_get(sc->sc_syncdev_ifidx);
+ if (syncdev && ISSET(sc->sc_if.if_flags, IFF_RUNNING)) {
/* Request a full state table update. */
sc->sc_ureq_sent = time_uptime;
#if NCARP > 0
@@ -1907,6 +1943,7 @@ pfsync_request_full_update(struct pfsync
sizeof(struct pfsync_state)));
pfsync_request_update(0, 0);
}
+ if_put(syncdev);
}
void
