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;
>
