On Tue, Oct 10, 2017 at 03:55:30PM +0200, Martin Pieuchot wrote:
> Similar diff without factorizing privilege checks, deraadt@ pointed out
> it is too fragile.
> 
> ok?

OK bluhm@

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

Reply via email to