diff options
author | ru <ru@FreeBSD.org> | 2005-11-24 18:56:14 +0000 |
---|---|---|
committer | ru <ru@FreeBSD.org> | 2005-11-24 18:56:14 +0000 |
commit | fd2f0452fd92ba735cd646e5b1a20def1a72d622 (patch) | |
tree | fdcf6716785fd45a03a8574e287608594ac0e359 | |
parent | 5d85d3ca54fa8a1a7c330ce11a6682b88c5c1499 (diff) | |
download | FreeBSD-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.
-rw-r--r-- | sys/net/if_clone.c | 25 |
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)); |