From owner-dev-commits-src-all@freebsd.org Sun May 23 00:04:17 2021 Return-Path: Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 5BC1F63AACD; Sun, 23 May 2021 00:04:17 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4FngYs1y2mz3hLQ; Sun, 23 May 2021 00:04:17 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 29B6C57C4; Sun, 23 May 2021 00:04:17 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 14N04H9b060521; Sun, 23 May 2021 00:04:17 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 14N04HMk060520; Sun, 23 May 2021 00:04:17 GMT (envelope-from git) Date: Sun, 23 May 2021 00:04:17 GMT Message-Id: <202105230004.14N04HMk060520@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Eugene Grosbein Subject: git: 18fa0cbfc4e9 - stable/12 - MFC r351629: sys/net/if_vlan.c: Wrap a vlan's parent's if_output in a separate function. MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: eugen X-Git-Repository: src X-Git-Refname: refs/heads/stable/12 X-Git-Reftype: branch X-Git-Commit: 18fa0cbfc4e906fbf824651140f68d0a85c1d08f Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 23 May 2021 00:04:17 -0000 The branch stable/12 has been updated by eugen: URL: https://cgit.FreeBSD.org/src/commit/?id=18fa0cbfc4e906fbf824651140f68d0a85c1d08f commit 18fa0cbfc4e906fbf824651140f68d0a85c1d08f Author: Matt Joras AuthorDate: 2019-08-30 20:19:43 +0000 Commit: Eugene Grosbein CommitDate: 2021-05-22 23:59:40 +0000 MFC r351629: sys/net/if_vlan.c: Wrap a vlan's parent's if_output in a separate function. The merge is done in preparation of another merge to support 802.1ad (qinq). Original commit log follows. 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 (cherry picked from commit 16cf6bdbb6cb18a5af7b499034b2176a1fa0a503) --- sys/net/if_vlan.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/sys/net/if_vlan.c b/sys/net/if_vlan.c index 1f5a95cdda7c..450c4b9bbe3b 100644 --- a/sys/net/if_vlan.c +++ b/sys/net/if_vlan.c @@ -291,6 +291,8 @@ static int vlan_setflag(struct ifnet *ifp, int flag, int status, static int vlan_setflags(struct ifnet *ifp, int status); static int vlan_setmulti(struct ifnet *ifp); 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_locked(struct ifnet *ifp, int departing); static int vlan_config(struct ifvlan *ifv, struct ifnet *p, uint16_t tag); @@ -1179,6 +1181,27 @@ vlan_transmit(struct ifnet *ifp, struct mbuf *m) 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. */ @@ -1392,13 +1415,18 @@ vlan_config(struct ifvlan *ifv, struct ifnet *p, uint16_t vid) */ ifp->if_mtu = p->if_mtu - ifv->ifv_mtufudge; ifp->if_baudrate = p->if_baudrate; - ifp->if_output = p->if_output; ifp->if_input = p->if_input; ifp->if_resolvemulti = p->if_resolvemulti; ifp->if_addrlen = p->if_addrlen; ifp->if_broadcastaddr = p->if_broadcastaddr; 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. * Other flags are none of our business.