On Mon, Feb 06, 2017 at 03:54:56PM +0100, Martin Pieuchot wrote:
> On 06/02/17(Mon) 12:27, Martin Pieuchot wrote:
> > Paul and Antoine reported that, since the NET_LOCK() went in, doing a
> > channel scan with iwm(4) and iwn(4) freezes X.
> >
> > This is a deadlock due to the fact that wireless drivers sleep during
> > a long time holding the NET_LOCK() while the X server tries to
> > communicate with unix sockets, that still need the NET_LOCK().
> >
> > The obvious solution to this problem is to not hold the NET_LOCK() for
> > unix socket related operations. We're working on that.
> >
> > However since hardware ioctl(2) can sleep there's no problem to release
> > the NET_LOCK() there. This is true for all ioctls that are not
> > modifying any stack state (address, multicast group, etc). Hence the
> > diff below that should fix the problem in a generic way.
>
> tb@ found that another code path needs to be unlocked. Diff below does
> that. These ioctl(2)s are only messing at the driver level, so it is
> safe to drop the NET_LOCK() there as well.
>
> ok?
OK pirofti@. Thanks!
>
> Index: netinet/in.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/in.c,v
> retrieving revision 1.133
> diff -u -p -r1.133 in.c
> --- netinet/in.c 20 Dec 2016 12:35:38 -0000 1.133
> +++ netinet/in.c 6 Feb 2017 13:43:06 -0000
> @@ -390,7 +390,11 @@ in_ioctl(u_long cmd, caddr_t data, struc
> default:
> if (ifp->if_ioctl == NULL)
> return (EOPNOTSUPP);
> - return ((*ifp->if_ioctl)(ifp, cmd, data));
> + /* XXXSMP breaks atomicity */
> + rw_exit_write(&netlock);
> + error = ((*ifp->if_ioctl)(ifp, cmd, data));
> + rw_enter_write(&netlock);
> + return (error);
> }
> return (0);
> }
> Index: netinet6/in6.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/in6.c,v
> retrieving revision 1.197
> diff -u -p -r1.197 in6.c
> --- netinet6/in6.c 5 Feb 2017 16:04:14 -0000 1.197
> +++ netinet6/in6.c 6 Feb 2017 13:43:06 -0000
> @@ -190,6 +190,7 @@ in6_ioctl(u_long cmd, caddr_t data, stru
> struct in6_ifaddr *ia6 = NULL;
> struct in6_aliasreq *ifra = (struct in6_aliasreq *)data;
> struct sockaddr_in6 *sa6;
> + int error;
>
> if (ifp == NULL)
> return (EOPNOTSUPP);
> @@ -469,9 +470,13 @@ in6_ioctl(u_long cmd, caddr_t data, stru
> break;
>
> default:
> - if (ifp == NULL || ifp->if_ioctl == 0)
> + if (ifp->if_ioctl == NULL)
> return (EOPNOTSUPP);
> - return ((*ifp->if_ioctl)(ifp, cmd, data));
> + /* XXXSMP breaks atomicity */
> + rw_exit_write(&netlock);
> + error = ((*ifp->if_ioctl)(ifp, cmd, data));
> + rw_enter_write(&netlock);
> + return (error);
> }
>
> return (0);
> Index: dev/pci/if_iwm.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
> retrieving revision 1.162
> diff -u -p -r1.162 if_iwm.c
> --- dev/pci/if_iwm.c 4 Feb 2017 19:20:59 -0000 1.162
> +++ dev/pci/if_iwm.c 6 Feb 2017 14:52:24 -0000
> @@ -6133,18 +6133,13 @@ iwm_ioctl(struct ifnet *ifp, u_long cmd,
> struct ifreq *ifr;
> int s, err = 0;
>
> - /* XXXSMP breaks atomicity */
> - rw_exit_write(&netlock);
> -
> /*
> * Prevent processes from entering this function while another
> * process is tsleep'ing in it.
> */
> err = rw_enter(&sc->ioctl_rwl, RW_WRITE | RW_INTR);
> - if (err) {
> - rw_enter_write(&netlock);
> + if (err)
> return err;
> - }
>
> s = splnet();
>
> @@ -6190,7 +6185,6 @@ iwm_ioctl(struct ifnet *ifp, u_long cmd,
>
> splx(s);
> rw_exit(&sc->ioctl_rwl);
> - rw_enter_write(&netlock);
>
> return err;
> }
> Index: net/if.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.485
> diff -u -p -r1.485 if.c
> --- net/if.c 1 Feb 2017 02:02:01 -0000 1.485
> +++ net/if.c 6 Feb 2017 14:27:47 -0000
> @@ -2029,7 +2029,10 @@ ifioctl(struct socket *so, u_long cmd, c
> case SIOCGIFPARENT:
> if (ifp->if_ioctl == 0)
> return (EOPNOTSUPP);
> + /* XXXSMP breaks atomicity */
> + rw_exit_write(&netlock);
> error = (*ifp->if_ioctl)(ifp, cmd, data);
> + rw_enter_write(&netlock);
> break;
>
> case SIOCGIFDESCR:
