Here's a fixed version of my previous diff that does not accidentally
add a walk over the ifa_list to SIOCSIFADDR.
Instead, start with a merged version of SIOCSIFADDR, add a copy of the
walk to both SIOCAIFADDR and SIOCDIFADDR and add the allocation of the
ia to SIACAIFADDR. As before, start each case with a privilege check.
Index: sys/netinet/in.c
===================================================================
RCS file: /var/cvs/src/sys/netinet/in.c,v
retrieving revision 1.157
diff -u -p -r1.157 in.c
--- sys/netinet/in.c 31 May 2018 17:17:01 -0000 1.157
+++ sys/netinet/in.c 31 May 2018 17:20:17 -0000
@@ -334,26 +334,10 @@ in_ioctl_change_ifaddr(u_long cmd, caddr
}
switch (cmd) {
- case SIOCAIFADDR:
- case SIOCDIFADDR:
- if (ifra->ifra_addr.sin_family == AF_INET) {
- for (; ifa != NULL; ifa = TAILQ_NEXT(ifa, ifa_list)) {
- if ((ifa->ifa_addr->sa_family == AF_INET) &&
- ifatoia(ifa)->ia_addr.sin_addr.s_addr ==
- ifra->ifra_addr.sin_addr.s_addr)
- break;
- }
- ia = ifatoia(ifa);
- }
- if (cmd == SIOCDIFADDR && ia == NULL) {
- error = EADDRNOTAVAIL;
- goto err;
- }
- /* FALLTHROUGH */
case SIOCSIFADDR:
if (!privileged) {
error = EPERM;
- goto err;
+ break;
}
if (ia == NULL) {
@@ -373,10 +357,7 @@ in_ioctl_change_ifaddr(u_long cmd, caddr
newifaddr = 1;
} else
newifaddr = 0;
- break;
- }
- switch(cmd) {
- case SIOCSIFADDR:
+
in_ifscrub(ifp, ia);
error = in_ifinit(ifp, ia, satosin(&ifr->ifr_addr), newifaddr);
if (error)
@@ -387,6 +368,38 @@ in_ioctl_change_ifaddr(u_long cmd, caddr
case SIOCAIFADDR: {
int needinit = 0;
+ if (!privileged) {
+ error = EPERM;
+ break;
+ }
+
+ if (ifra->ifra_addr.sin_family == AF_INET) {
+ for (; ifa != NULL; ifa = TAILQ_NEXT(ifa, ifa_list)) {
+ if ((ifa->ifa_addr->sa_family == AF_INET) &&
+ ifatoia(ifa)->ia_addr.sin_addr.s_addr ==
+ ifra->ifra_addr.sin_addr.s_addr)
+ break;
+ }
+ ia = ifatoia(ifa);
+ }
+ if (ia == NULL) {
+ ia = malloc(sizeof *ia, M_IFADDR, M_WAITOK | M_ZERO);
+ ia->ia_addr.sin_family = AF_INET;
+ ia->ia_addr.sin_len = sizeof(ia->ia_addr);
+ ia->ia_ifa.ifa_addr = sintosa(&ia->ia_addr);
+ ia->ia_ifa.ifa_dstaddr = sintosa(&ia->ia_dstaddr);
+ ia->ia_ifa.ifa_netmask = sintosa(&ia->ia_sockmask);
+ ia->ia_sockmask.sin_len = 8;
+ if (ifp->if_flags & IFF_BROADCAST) {
+ ia->ia_broadaddr.sin_len = sizeof(ia->ia_addr);
+ ia->ia_broadaddr.sin_family = AF_INET;
+ }
+ ia->ia_ifp = ifp;
+
+ newifaddr = 1;
+ } else
+ newifaddr = 0;
+
if (ia->ia_addr.sin_family == AF_INET) {
if (ifra->ifra_addr.sin_len == 0)
ifra->ifra_addr = ia->ia_addr;
@@ -423,6 +436,25 @@ in_ioctl_change_ifaddr(u_long cmd, caddr
break;
}
case SIOCDIFADDR:
+ if (!privileged) {
+ error = EPERM;
+ break;
+ }
+
+ if (ifra->ifra_addr.sin_family == AF_INET) {
+ for (; ifa != NULL; ifa = TAILQ_NEXT(ifa, ifa_list)) {
+ if ((ifa->ifa_addr->sa_family == AF_INET) &&
+ ifatoia(ifa)->ia_addr.sin_addr.s_addr ==
+ ifra->ifra_addr.sin_addr.s_addr)
+ break;
+ }
+ ia = ifatoia(ifa);
+ }
+ if (ia == NULL) {
+ error = EADDRNOTAVAIL;
+ break;
+ }
+
/*
* Even if the individual steps were safe, shouldn't
* these kinds of changes happen atomically? What
@@ -437,7 +469,6 @@ in_ioctl_change_ifaddr(u_long cmd, caddr
panic("invalid ioctl %lu", cmd);
}
-err:
NET_UNLOCK();
return (error);
}
