Hello and thanks for the report.
On 21/09/18(Fri) 23:06, [email protected] wrote:
> >Synopsis: Panic when destroying vlan interface with virtio parent in
> >promisc mode
> >Category: kernel (net)
> >Environment:
> System : OpenBSD 6.3
> Details : OpenBSD 6.3-stable (GENERIC.MP) #0: Fri Sep 21 19:31:55
> CEST 2018
>
> root@openbsd:/usr/src/sys/arch/amd64/compile/GENERIC.MP
>
> Architecture: OpenBSD.amd64
> Machine : amd64
> >Description:
> Destroying a vlan interface when parent is a virtio interface in
> promiscuous
> mode trigger a panic (netlock: lock no held). Indeed, vlan_clone_destroy() is
> called
> without netlock, but when the parent interface is in promiscuous mode, there
> is a call
> to ifpromisc. The virtio interface use rwsleep() which expects the netlock
> held.
> >How-To-Repeat:
> ifconfig vlan100 vnetid 100 parent vio0 lladdr 00:11:22:33:44:55 up
> ifconfig vlan100 destroy
> >Fix:
> Add NET_LOCK() / NET_UNLOCK() around vlan_down() in
> vlan_clone_destroy() in
> net/if_vlan.c.
The problem is in vio(4), the diff below should fix it. Could you try
it?
Index: dev/pv/if_vio.c
===================================================================
RCS file: /cvs/src/sys/dev/pv/if_vio.c,v
retrieving revision 1.5
diff -u -p -r1.5 if_vio.c
--- dev/pv/if_vio.c 27 Feb 2018 08:44:58 -0000 1.5
+++ dev/pv/if_vio.c 24 Sep 2018 12:22:20 -0000
@@ -1270,14 +1270,28 @@ out:
return r;
}
+/*
+ * XXXSMP As long as some per-ifp ioctl(2)s are executed with the
+ * NET_LOCK() deadlocks are possible. So release it here.
+ */
+static inline int
+vio_sleep(struct vio_softc *sc, const char *wmesg)
+{
+ int status = rw_status(&netlock);
+
+ if (status != RW_WRITE && status != RW_READ)
+ return tsleep(&sc->sc_ctrl_inuse, PRIBIO|PCATCH, wmesg, 0);
+
+ return rwsleep(&sc->sc_ctrl_inuse, &netlock, PRIBIO|PCATCH, wmesg, 0);
+}
+
int
vio_wait_ctrl(struct vio_softc *sc)
{
int r = 0;
while (sc->sc_ctrl_inuse != FREE) {
- r = rwsleep(&sc->sc_ctrl_inuse, &netlock, PRIBIO|PCATCH,
- "viowait", 0);
+ r = vio_sleep(sc, "viowait");
if (r == EINTR)
return r;
}
@@ -1296,8 +1310,7 @@ vio_wait_ctrl_done(struct vio_softc *sc)
r = 1;
break;
}
- r = rwsleep(&sc->sc_ctrl_inuse, &netlock, PRIBIO|PCATCH,
- "viodone", 0);
+ r = vio_sleep(sc, "viodone");
if (r == EINTR)
break;
}
