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

Reply via email to