d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Wrap a vlan's parent's if_output in a separate function.

Browse Source 
When a vlan interface is created, its if_output is set directly to the
parent interface's if_output. This is fine in the normal case but has an
unfortunate consequence if you end up with a certain combination of vlan
and lagg interfaces.

Consider you have a lagg interface with a single laggport member. When
an interface is added to a lagg its if_output is set to
lagg_port_output, which blackholes traffic from the normal networking
stack but not certain frames from BPF (pseudo_AF_HDRCMPLT). If you now
create a vlan with the laggport member (not the lagg interface) as its
parent, its if_output is set to lagg_port_output as well. While this is
confusing conceptually and likely represents a misconfigured system, it
is not itself a problem. The problem arises when you then remove the
lagg interface. Doing this resets the if_output of the laggport member
back to its original state, but the vlan's if_output is left pointing to
lagg_port_output. This gives rise to the possibility that the system
will panic when e.g. bpf is used to send any frames on the vlan
interface.

Fix this by creating a new function, vlan_output, which simply wraps the
parent's current if_output. That way when the parent's if_output is
restored there is no stale usage of lagg_port_output.

Reviewed by:	rstone
Differential Revision:	D21209
This commit is contained in:
Matt Joras 2019-08-30 20:19:43 +00:00
parent d794b3a3c2
commit 16cf6bdbb6
1 changed files with 29 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
sys/net/if_vlan.c
View File

@ -294,6 +294,8 @@ static int vlan_setflag(struct ifnet *ifp, int flag, int status,
static int vlan_setflags(struct ifnet *ifp, int status); static int vlan_setflags(struct ifnet *ifp, int status);
static int vlan_setmulti(struct ifnet *ifp); static int vlan_setmulti(struct ifnet *ifp);
static int vlan_transmit(struct ifnet *ifp, struct mbuf *m); static int vlan_transmit(struct ifnet *ifp, struct mbuf *m);
static int vlan_output(struct ifnet *ifp, struct mbuf *m,
const struct sockaddr *dst, struct route *ro);
static void vlan_unconfig(struct ifnet *ifp); static void vlan_unconfig(struct ifnet *ifp);
static void vlan_unconfig_locked(struct ifnet *ifp, int departing); static void vlan_unconfig_locked(struct ifnet *ifp, int departing);
static int vlan_config(struct ifvlan *ifv, struct ifnet *p, uint16_t tag); static int vlan_config(struct ifvlan *ifv, struct ifnet *p, uint16_t tag);
@ -1209,6 +1211,27 @@ vlan_transmit(struct ifnet *ifp, struct mbuf *m)
return (error); return (error);
} }
static int
vlan_output(struct ifnet *ifp, struct mbuf *m, const struct sockaddr *dst,
struct route *ro)
{
struct epoch_tracker et;
struct ifvlan *ifv;
struct ifnet *p;
NET_EPOCH_ENTER(et);
ifv = ifp->if_softc;
if (TRUNK(ifv) == NULL) {
NET_EPOCH_EXIT(et);
m_freem(m);
return (ENETDOWN);
}
p = PARENT(ifv);
NET_EPOCH_EXIT(et);
return p->if_output(ifp, m, dst, ro);
}
/* /*
* The ifp->if_qflush entry point for vlan(4) is a no-op. * The ifp->if_qflush entry point for vlan(4) is a no-op.
*/ */
@ -1424,13 +1447,18 @@ vlan_config(struct ifvlan *ifv, struct ifnet *p, uint16_t vid)
*/ */
ifp->if_mtu = p->if_mtu - ifv->ifv_mtufudge; ifp->if_mtu = p->if_mtu - ifv->ifv_mtufudge;
ifp->if_baudrate = p->if_baudrate; ifp->if_baudrate = p->if_baudrate;
ifp->if_output = p->if_output;
ifp->if_input = p->if_input; ifp->if_input = p->if_input;
ifp->if_resolvemulti = p->if_resolvemulti; ifp->if_resolvemulti = p->if_resolvemulti;
ifp->if_addrlen = p->if_addrlen; ifp->if_addrlen = p->if_addrlen;
ifp->if_broadcastaddr = p->if_broadcastaddr; ifp->if_broadcastaddr = p->if_broadcastaddr;
ifp->if_pcp = ifv->ifv_pcp; ifp->if_pcp = ifv->ifv_pcp;
/*
* We wrap the parent's if_output using vlan_output to ensure that it
* can't become stale.
*/
ifp->if_output = vlan_output;
/* /*
* Copy only a selected subset of flags from the parent. * Copy only a selected subset of flags from the parent.
* Other flags are none of our business. * Other flags are none of our business.