summaryrefslogtreecommitdiffstats
path: root/sys
diff options
context:
space:
mode:
authorru <ru@FreeBSD.org>2005-11-24 18:56:14 +0000
committerru <ru@FreeBSD.org>2005-11-24 18:56:14 +0000
commitfd2f0452fd92ba735cd646e5b1a20def1a72d622 (patch)
treefdcf6716785fd45a03a8574e287608594ac0e359 /sys
parent5d85d3ca54fa8a1a7c330ce11a6682b88c5c1499 (diff)
downloadFreeBSD-src-fd2f0452fd92ba735cd646e5b1a20def1a72d622.zip
FreeBSD-src-fd2f0452fd92ba735cd646e5b1a20def1a72d622.tar.gz
Fix the following bugs:
- In ifc_name2unit(), disallow leading zeroes in a unit. Exploit: ifconfig lo01 create - In ifc_name2unit(), properly handle overflows. Otherwise, either of two local panic()'s can occur, either because no interface with such a name could be found after it was successfully created, or because the code will bogusly assume that it's a wildcard (unit < 0 due to overflow). Exploit: ifconfig lo<overflowed_integer> create - Previous revision made the following sequence trigger a KASSERT() failure in queue(3): Exploit: ifconfig lo0 destroy; ifconfig lo0 destroy This is because IFC_IFLIST_REMOVE() is always called before ifc->ifc_destroy() has been run, not accounting for the fact that the latter can fail and leave the interface operating (like is the case for "lo0"). So we ended up calling LIST_REMOVE() twice. We cannot defer IFC_IFLIST_REMOVE() until after a call to ifc->ifc_destroy() because the ifnet may have been removed and its memory has been freed, so recover from this by re-inserting the ifnet in the cloned interfaces list if ifc->ifc_destroy() indicates a failure.
Diffstat (limited to 'sys')
-rw-r--r--sys/net/if_clone.c25
1 files changed, 20 insertions, 5 deletions
diff --git a/sys/net/if_clone.c b/sys/net/if_clone.c
index 859ece2..89d679d 100644
--- a/sys/net/if_clone.c
+++ b/sys/net/if_clone.c
@@ -32,6 +32,7 @@
#include <sys/param.h>
#include <sys/malloc.h>
+#include <sys/limits.h>
#include <sys/lock.h>
#include <sys/mutex.h>
#include <sys/kernel.h>
@@ -200,17 +201,23 @@ if_clone_destroyif(struct if_clone *ifc, struct ifnet *ifp)
{
int err;
- IF_CLONE_LOCK(ifc);
- IFC_IFLIST_REMOVE(ifc, ifp);
- IF_CLONE_UNLOCK(ifc);
-
if (ifc->ifc_destroy == NULL) {
err = EOPNOTSUPP;
goto done;
}
+ IF_CLONE_LOCK(ifc);
+ IFC_IFLIST_REMOVE(ifc, ifp);
+ IF_CLONE_UNLOCK(ifc);
+
err = (*ifc->ifc_destroy)(ifc, ifp);
+ if (err != 0) {
+ IF_CLONE_LOCK(ifc);
+ IFC_IFLIST_INSERT(ifc, ifp);
+ IF_CLONE_UNLOCK(ifc);
+ }
+
done:
return (err);
}
@@ -349,16 +356,24 @@ int
ifc_name2unit(const char *name, int *unit)
{
const char *cp;
+ int cutoff = INT_MAX / 10;
+ int cutlim = INT_MAX % 10;
for (cp = name; *cp != '\0' && (*cp < '0' || *cp > '9'); cp++);
if (*cp == '\0') {
*unit = -1;
+ } else if (cp[0] == '0' && cp[1] != '\0') {
+ /* Disallow leading zeroes. */
+ return (EINVAL);
} else {
for (*unit = 0; *cp != '\0'; cp++) {
if (*cp < '0' || *cp > '9') {
/* Bogus unit number. */
return (EINVAL);
}
+ if (*unit > cutoff ||
+ (*unit == cutoff && *cp - '0' > cutlim))
+ return (EINVAL);
*unit = (*unit * 10) + (*cp - '0');
}
}
@@ -447,7 +462,7 @@ ifc_simple_attach(struct if_clone *ifc)
struct ifc_simple_data *ifcs = ifc->ifc_data;
KASSERT(ifcs->ifcs_minifs - 1 <= ifc->ifc_maxunit,
- ("%s: %s requested more units then allowed (%d > %d)",
+ ("%s: %s requested more units than allowed (%d > %d)",
__func__, ifc->ifc_name, ifcs->ifcs_minifs,
ifc->ifc_maxunit + 1));
OpenPOWER on IntegriCloud