On Mon, Oct 09, 2017 at 04:34:38PM +0200, 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. > > ok?
Before the cmds SIOCAIFGROUP SIOCDIFGROUP SIOCSIFLLADDR SIOCSIFLLPRIO returned EPERM, with this patch 1.. Is this a problem? Regards, Michele > > 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 9 Oct 2017 14:27:35 -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,17 +1827,49 @@ ifioctl(struct socket *so, u_long cmd, c > const char *label; > > switch (cmd) { > - > - case SIOCGIFCONF: > - return (ifconf(cmd, data)); > + case SIOCIFCREATE: > + case SIOCIFDESTROY: > + case SIOCSIFGATTR: > + case SIOCIFAFATTACH: > + case SIOCIFAFDETACH: > + case SIOCSIFFLAGS: > + case SIOCSIFXFLAGS: > + case SIOCSIFMETRIC: > + case SIOCSIFMTU: > + case SIOCSIFPHYADDR: > + case SIOCDIFPHYADDR: > +#ifdef INET6 > + case SIOCSIFPHYADDR_IN6: > +#endif > + case SIOCSLIFPHYADDR: > + case SIOCSLIFPHYRTABLE: > + case SIOCSLIFPHYTTL: > + case SIOCADDMULTI: > + case SIOCDELMULTI: > + case SIOCSIFMEDIA: > + case SIOCSVNETID: > + case SIOCSIFPAIR: > + case SIOCSIFPARENT: > + case SIOCDIFPARENT: > + case SIOCSIFDESCR: > + case SIOCSIFRTLABEL: > + case SIOCSIFPRIORITY: > + case SIOCSIFRDOMAIN: > + case SIOCAIFGROUP: > + case SIOCDIFGROUP: > + case SIOCSIFLLADDR: > + case SIOCSIFLLPRIO: > + if ((error = suser(p, 0)) != 0) > + return (error); > + default: > + break; > } > - ifr = (struct ifreq *)data; > > switch (cmd) { > + case SIOCGIFCONF: > + return (ifconf(cmd, data)); > case SIOCIFCREATE: > case SIOCIFDESTROY: > - if ((error = suser(p, 0)) != 0) > - return (error); > return ((cmd == SIOCIFCREATE) ? > if_clone_create(ifr->ifr_name, 0) : > if_clone_destroy(ifr->ifr_name)); > @@ -1849,17 +1880,17 @@ ifioctl(struct socket *so, u_long cmd, c > case SIOCGIFGATTR: > return (if_getgroupattribs(data)); > case SIOCSIFGATTR: > - 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; > + > + 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: > @@ -1880,15 +1911,7 @@ ifioctl(struct socket *so, u_long cmd, c > } > 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; > if (ifq_is_oactive(&ifp->if_snd)) > @@ -1919,9 +1942,6 @@ ifioctl(struct socket *so, u_long cmd, c > } > > case SIOCSIFFLAGS: > - if ((error = suser(p, 0)) != 0) > - return (error); > - > ifp->if_flags = (ifp->if_flags & IFF_CANTCHANGE) | > (ifr->ifr_flags & ~IFF_CANTCHANGE); > > @@ -1944,9 +1964,6 @@ ifioctl(struct socket *so, u_long cmd, c > break; > > case SIOCSIFXFLAGS: > - if ((error = suser(p, 0)) != 0) > - return (error); > - > #ifdef INET6 > if (ISSET(ifr->ifr_flags, IFXF_AUTOCONF6)) { > error = in6_ifattach(ifp); > @@ -2006,14 +2023,10 @@ ifioctl(struct socket *so, u_long cmd, c > break; > > case SIOCSIFMETRIC: > - if ((error = suser(p, 0)) != 0) > - return (error); > ifp->if_metric = ifr->ifr_metric; > break; > > case SIOCSIFMTU: > - if ((error = suser(p, 0)) != 0) > - return (error); > if (ifp->if_ioctl == NULL) > return (EOPNOTSUPP); > error = (*ifp->if_ioctl)(ifp, cmd, data); > @@ -2036,9 +2049,6 @@ ifioctl(struct socket *so, u_long cmd, c > case SIOCSIFPAIR: > case SIOCSIFPARENT: > case SIOCDIFPARENT: > - if ((error = suser(p, 0)) != 0) > - return (error); > - /* FALLTHROUGH */ > case SIOCGIFPSRCADDR: > case SIOCGIFPDSTADDR: > case SIOCGLIFPHYADDR: > @@ -2060,8 +2070,6 @@ ifioctl(struct socket *so, u_long cmd, c > break; > > case SIOCSIFDESCR: > - if ((error = suser(p, 0)) != 0) > - return (error); > error = copyinstr(ifr->ifr_data, ifdescrbuf, > IFDESCRSIZE, &bytesdone); > if (error == 0) { > @@ -2081,8 +2089,6 @@ ifioctl(struct socket *so, u_long cmd, c > break; > > case SIOCSIFRTLABEL: > - if ((error = suser(p, 0)) != 0) > - return (error); > error = copyinstr(ifr->ifr_data, ifrtlabelbuf, > RTLABEL_LEN, &bytesdone); > if (error == 0) { > @@ -2096,8 +2102,6 @@ ifioctl(struct socket *so, u_long cmd, c > break; > > case SIOCSIFPRIORITY: > - if ((error = suser(p, 0)) != 0) > - return (error); > if (ifr->ifr_metric < 0 || ifr->ifr_metric > 15) > return (EINVAL); > ifp->if_priority = ifr->ifr_metric; > @@ -2108,15 +2112,11 @@ ifioctl(struct socket *so, u_long cmd, c > break; > > case SIOCSIFRDOMAIN: > - if ((error = suser(p, 0)) != 0) > - return (error); > if ((error = if_setrdomain(ifp, ifr->ifr_rdomainid)) != 0) > return (error); > break; > > case SIOCAIFGROUP: > - if ((error = suser(p, 0))) > - return (error); > ifgr = (struct ifgroupreq *)data; > if ((error = if_addgroup(ifp, ifgr->ifgr_group))) > return (error); > @@ -2129,8 +2129,6 @@ ifioctl(struct socket *so, u_long cmd, c > break; > > case SIOCDIFGROUP: > - if ((error = suser(p, 0))) > - return (error); > (*ifp->if_ioctl)(ifp, cmd, data); /* XXX error check */ > ifgr = (struct ifgroupreq *)data; > if ((error = if_delgroup(ifp, ifgr->ifgr_group))) > @@ -2138,10 +2136,7 @@ ifioctl(struct socket *so, u_long cmd, c > break; > > 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); > @@ -2169,8 +2164,6 @@ ifioctl(struct socket *so, u_long cmd, c > break; > > case SIOCSIFLLPRIO: > - if ((error = suser(p, 0))) > - return (error); > if (ifr->ifr_llprio > UCHAR_MAX) > return (EINVAL); > ifp->if_llprio = ifr->ifr_llprio; >