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