Hello,
On 9/24/18 2:23 PM, Martin Pieuchot wrote:
> 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?
Successfully tested, thanks for the fix.
> 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;
> }
>
