On 09/10/17(Mon) 16:34, Martin Pieuchot wrote:
> Here's a small cleanup to make it easy to push the NET_LOCK() further
> down.
>
> Diff below factorize all ioctl requests need root privileges in one
> switch () block.
>
> It moves SIOCIFAFATTACH/SIOCIFAFDETACH where it belongs, to the block
> that keeps track of the timestamps of the last change.
>
> It gets rid of unnecessary `sdl' variable.
Similar diff without factorizing privilege checks, deraadt@ pointed out
it is too fragile.
ok?
Index: net/if.c
===================================================================
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.513
diff -u -p -r1.513 if.c
--- net/if.c 9 Oct 2017 08:35:38 -0000 1.513
+++ net/if.c 10 Oct 2017 13:46:43 -0000
@@ -1816,10 +1816,9 @@ int
ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p)
{
struct ifnet *ifp;
- struct ifreq *ifr;
- struct sockaddr_dl *sdl;
+ struct ifreq *ifr = (struct ifreq *)data;
struct ifgroupreq *ifgr;
- struct if_afreq *ifar;
+ struct if_afreq *ifar = (struct if_afreq *)data;
char ifdescrbuf[IFDESCRSIZE];
char ifrtlabelbuf[RTLABEL_LEN];
int s, error = 0, oif_xflags;
@@ -1828,13 +1827,8 @@ ifioctl(struct socket *so, u_long cmd, c
const char *label;
switch (cmd) {
-
case SIOCGIFCONF:
return (ifconf(cmd, data));
- }
- ifr = (struct ifreq *)data;
-
- switch (cmd) {
case SIOCIFCREATE:
case SIOCIFDESTROY:
if ((error = suser(p, 0)) != 0)
@@ -1852,15 +1846,19 @@ ifioctl(struct socket *so, u_long cmd, c
if ((error = suser(p, 0)) != 0)
return (error);
return (if_setgroupattribs(data));
+ }
+
+ ifp = ifunit(ifr->ifr_name);
+ if (ifp == NULL)
+ return (ENXIO);
+ oif_flags = ifp->if_flags;
+ oif_xflags = ifp->if_xflags;
+
+ switch (cmd) {
case SIOCIFAFATTACH:
case SIOCIFAFDETACH:
if ((error = suser(p, 0)) != 0)
return (error);
- ifar = (struct if_afreq *)data;
- if ((ifp = ifunit(ifar->ifar_name)) == NULL)
- return (ENXIO);
- oif_flags = ifp->if_flags;
- oif_xflags = ifp->if_xflags;
switch (ifar->ifar_af) {
case AF_INET:
/* attach is a noop for AF_INET */
@@ -1878,16 +1876,7 @@ ifioctl(struct socket *so, u_long cmd, c
default:
return (EAFNOSUPPORT);
}
- if (oif_flags != ifp->if_flags || oif_xflags != ifp->if_xflags)
- rtm_ifchg(ifp);
- return (error);
- }
-
- ifp = ifunit(ifr->ifr_name);
- if (ifp == 0)
- return (ENXIO);
- oif_flags = ifp->if_flags;
- switch (cmd) {
+ break;
case SIOCGIFFLAGS:
ifr->ifr_flags = ifp->if_flags;
@@ -2002,7 +1991,6 @@ ifioctl(struct socket *so, u_long cmd, c
ifp->if_xflags = (ifp->if_xflags & IFXF_CANTCHANGE) |
(ifr->ifr_flags & ~IFXF_CANTCHANGE);
- rtm_ifchg(ifp);
break;
case SIOCSIFMETRIC:
@@ -2140,8 +2128,7 @@ ifioctl(struct socket *so, u_long cmd, c
case SIOCSIFLLADDR:
if ((error = suser(p, 0)))
return (error);
- sdl = ifp->if_sadl;
- if (sdl == NULL)
+ if (ifp->if_sadl == NULL)
return (EINVAL);
if (ifr->ifr_addr.sa_len != ETHER_ADDR_LEN)
return (EINVAL);
@@ -2182,6 +2169,9 @@ ifioctl(struct socket *so, u_long cmd, c
(struct mbuf *) ifp, p));
break;
}
+
+ if (oif_flags != ifp->if_flags || oif_xflags != ifp->if_xflags)
+ rtm_ifchg(ifp);
if (((oif_flags ^ ifp->if_flags) & IFF_UP) != 0)
getmicrotime(&ifp->if_lastchange);
