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.
This commit is contained in:
parent
5d85d3ca54
commit
fd2f0452fd
@ -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));
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user