Author: kp
Date: Fri Jan 31 10:34:38 2020
New Revision: 357326
URL: https://svnweb.freebsd.org/changeset/base/357326

Log:
  MFC r357233:
  
  epair: Do not abuse params to register the second interface
  
  if_epair used the 'params' argument to pass a pointer to the b interface
  through if_clone_create().
  This pointer can be controlled by userspace, which means it could be abused to
  trigger a panic. While this requires PRIV_NET_IFCREATE
  privileges those are assigned to vnet jails, which means that vnet jails
  could panic the system.
  
  Reported by:  Ilja Van Sprundel <[email protected]>

Modified:
  stable/11/sys/net/if_clone.c
  stable/11/sys/net/if_clone.h
  stable/11/sys/net/if_epair.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/sys/net/if_clone.c
==============================================================================
--- stable/11/sys/net/if_clone.c        Fri Jan 31 10:34:36 2020        
(r357325)
+++ stable/11/sys/net/if_clone.c        Fri Jan 31 10:34:38 2020        
(r357326)
@@ -208,6 +208,17 @@ if_clone_create(char *name, size_t len, caddr_t params
        return (if_clone_createif(ifc, name, len, params));
 }
 
+void
+if_clone_addif(struct if_clone *ifc, struct ifnet *ifp)
+{
+
+       if_addgroup(ifp, ifc->ifc_name);
+
+       IF_CLONE_LOCK(ifc);
+       IFC_IFLIST_INSERT(ifc, ifp);
+       IF_CLONE_UNLOCK(ifc);
+}
+
 /*
  * Create a clone network interface.
  */
@@ -230,11 +241,7 @@ if_clone_createif(struct if_clone *ifc, char *name, si
                if (ifp == NULL)
                        panic("%s: lookup failed for %s", __func__, name);
 
-               if_addgroup(ifp, ifc->ifc_name);
-
-               IF_CLONE_LOCK(ifc);
-               IFC_IFLIST_INSERT(ifc, ifp);
-               IF_CLONE_UNLOCK(ifc);
+               if_clone_addif(ifc, ifp);
        }
 
        return (err);

Modified: stable/11/sys/net/if_clone.h
==============================================================================
--- stable/11/sys/net/if_clone.h        Fri Jan 31 10:34:36 2020        
(r357325)
+++ stable/11/sys/net/if_clone.h        Fri Jan 31 10:34:38 2020        
(r357326)
@@ -72,7 +72,8 @@ int   if_clone_list(struct if_clonereq *);
 struct if_clone *if_clone_findifc(struct ifnet *);
 void   if_clone_addgroup(struct ifnet *, struct if_clone *);
 
-/* The below interface used only by epair(4). */
+/* The below interfaces are used only by epair(4). */
+void   if_clone_addif(struct if_clone *, struct ifnet *);
 int    if_clone_destroyif(struct if_clone *, struct ifnet *);
 
 #endif /* _KERNEL */

Modified: stable/11/sys/net/if_epair.c
==============================================================================
--- stable/11/sys/net/if_epair.c        Fri Jan 31 10:34:36 2020        
(r357325)
+++ stable/11/sys/net/if_epair.c        Fri Jan 31 10:34:38 2020        
(r357326)
@@ -704,6 +704,23 @@ epair_clone_match(struct if_clone *ifc, const char *na
        return (1);
 }
 
+static void
+epair_clone_add(struct if_clone *ifc, struct epair_softc *scb)
+{
+       struct ifnet *ifp;
+       uint8_t eaddr[ETHER_ADDR_LEN];  /* 00:00:00:00:00:00 */
+
+       ifp = scb->ifp;
+       /* Assign a hopefully unique, locally administered etheraddr. */
+       eaddr[0] = 0x02;
+       eaddr[3] = (ifp->if_index >> 8) & 0xff;
+       eaddr[4] = ifp->if_index & 0xff;
+       eaddr[5] = 0x0b;
+       ether_ifattach(ifp, eaddr);
+
+       if_clone_addif(ifc, ifp);
+}
+
 static int
 epair_clone_create(struct if_clone *ifc, char *name, size_t len, caddr_t 
params)
 {
@@ -713,26 +730,6 @@ epair_clone_create(struct if_clone *ifc, char *name, s
        int error, unit, wildcard;
        uint8_t eaddr[ETHER_ADDR_LEN];  /* 00:00:00:00:00:00 */
 
-       /*
-        * We are abusing params to create our second interface.
-        * Actually we already created it and called if_clone_create()
-        * for it to do the official insertion procedure the moment we knew
-        * it cannot fail anymore. So just do attach it here.
-        */
-       if (params) {
-               scb = (struct epair_softc *)params;
-               ifp = scb->ifp;
-               /* Assign a hopefully unique, locally administered etheraddr. */
-               eaddr[0] = 0x02;
-               eaddr[3] = (ifp->if_index >> 8) & 0xff;
-               eaddr[4] = ifp->if_index & 0xff;
-               eaddr[5] = 0x0b;
-               ether_ifattach(ifp, eaddr);
-               /* Correctly set the name for the cloner list. */
-               strlcpy(name, scb->ifp->if_xname, len);
-               return (0);
-       }
-
        /* Try to see if a special unit was requested. */
        error = ifc_name2unit(name, &unit);
        if (error != 0)
@@ -860,10 +857,11 @@ epair_clone_create(struct if_clone *ifc, char *name, s
        ifp->if_snd.ifq_maxlen = ifqmaxlen;
        /* We need to play some tricks here for the second interface. */
        strlcpy(name, epairname, len);
-       error = if_clone_create(name, len, (caddr_t)scb);
-       if (error)
-               panic("%s: if_clone_create() for our 2nd iface failed: %d",
-                   __func__, error);
+
+       /* Correctly set the name for the cloner list. */
+       strlcpy(name, scb->ifp->if_xname, len);
+       epair_clone_add(ifc, scb);
+
        scb->if_qflush = ifp->if_qflush;
        ifp->if_qflush = epair_qflush;
        ifp->if_transmit = epair_transmit;
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"

Reply via email to