On Mon, Dec 18, 2017 at 04:01:12PM +0100, Martin Pieuchot wrote: > Diff below moves the NET_LOCK() into every switch case that need it and > document locking for most of the fields of 'struct ifnet'.
> case SIOCSIFXFLAGS: > if ((error = suser(p, 0)) != 0) > break; > > + NET_LOCK(); > #ifdef INET6 > if (ISSET(ifr->ifr_flags, IFXF_AUTOCONF6)) { > error = in6_ifattach(ifp); Just a line below is an "if (error != 0) break;". There the unlock is missing. > case SIOCSIFMTU: > if ((error = suser(p, 0)) != 0) > break; > + NET_LOCK(); > error = (*ifp->if_ioctl)(ifp, cmd, data); > + NET_UNLOCK(); > if (!error) > rtm_ifchg(ifp); > break; rtm_ifchg() calls route_input() which needs the net lock. > @@ -2103,8 +2118,6 @@ ifioctl(struct socket *so, u_long cmd, c > > if (((oif_flags ^ ifp->if_flags) & IFF_UP) != 0) > getmicrotime(&ifp->if_lastchange); > - > - NET_UNLOCK(); > > return (error); > } May we access the ifp->if_flags without net lock? As we want to compare them to their value before we changed them, would it be even better to put the "oif_flags = ifp->if_flags;" under the lock? What is the advantage to move the net lock into the cases? bluhm