Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 27 Nov 2019 18:35:23 +0000
From:      bugzilla-noreply@freebsd.org
To:        bugs@FreeBSD.org
Subject:   [Bug 242270] Network stack leaks ifnet references when creating VLAN
Message-ID:  <bug-242270-227@https.bugs.freebsd.org/bugzilla/>

next in thread | raw e-mail | index | archive | help
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D242270

            Bug ID: 242270
           Summary: Network stack leaks ifnet references when creating
                    VLAN
           Product: Base System
           Version: CURRENT
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Many People
          Priority: ---
         Component: kern
          Assignee: bugs@FreeBSD.org
          Reporter: aboyer@pensando.io

When a VLAN is created on a struct ifnet, one extra leaked reference is add=
ed
on the ifnet. This reference is never removed and results in a leak of the
ifnet once it is forgotten by the network stack.
Roughly:
 * if_clone_create() -> vlan_clone_match() -> vlan_clone_match_ethervid() t=
akes
a ref (1)
 * if_clone_createif() -> vlan_clone_create() -> vlan_clone_match_ethervid()
takes a ref (2)
 * if_clone_createif() -> vlan_clone_create() -> vlan_config() takes a ref =
(3)
 * if_clone_createif() -> vlan_clone_create() drops a ref (3)
 * One ref is dropped when the VLAN is destroyed (2)
 * Ref (1) is leaked
The first pass, if_clone_create(), is just to verify that an ifp named "foo=
0"
is present in the system. When if_clone_createif() goes back for the second
pass, it does the check again and errors out cleanly if the name is not
present. The refcounting was added to this code after it had been in the ke=
rnel
for years; prior to the refcounting, it would have been necessary for the
second pass to re-confirm that the port exists.
To me, it seems clear that the first pass was not intended to return with a
reference held. It doesn't return an ifp pointer or anything - just '1' to
indicate that the named interface exists. It would be a much larger redesig=
n to
combine everything into one pass. Instead we should just fix the reference
leak.

(This might only affect foo0.N style VLANs)

This is a fix:
--- a/sys/net/if_vlan.c
+++ b/sys/net/if_vlan.c
@@ -957,10 +957,14 @@ vlan_clone_match_ethervid(const char *name, int *vidp)
 static int
 vlan_clone_match(struct if_clone *ifc, const char *name)
 {
+       struct ifnet *ifp;
        const char *cp;

-       if (vlan_clone_match_ethervid(name, NULL) !=3D NULL)
+       ifp =3D vlan_clone_match_ethervid(name, NULL);
+       if (ifp !=3D NULL) {
+               if_rele(ifp);
                return (1);
+       }

        if (strncmp(vlanname, name, strlen(vlanname)) !=3D 0)
                return (0);

--=20
You are receiving this mail because:
You are the assignee for the bug.=



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-242270-227>