switch(4): fix netlock assertion within ifpromisc()

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

switch(4): fix netlock assertion within ifpromisc()

Vitaliy Makkoveev-2
As it was reported [1] switch(4) triggers NET_ASSERT_LOCKED() while
we perform ifconfig(8) destroy. ifpromisc() requires netlock to be held.
This is true while switch_port_detach() and underlay ifpromisc() called
through switch_ioctl(). But while we destroy switch(4) interface we call
ifpromisc() without netlock. We can't add netlock just around
ifpromisc() because switch_port_detach() which calls it called by
switch_ioctl() with netlock held and by switch_clone_destroy() without
netlock held. So the solution is to add netlock to destroy path. Diff
below adds it around the whole foreach loop where we call
switch_port_detach().

1. https://marc.info/?l=openbsd-bugs&m=161338077403538&w=2

Index: sys/net/if_switch.c
===================================================================
RCS file: /cvs/src/sys/net/if_switch.c,v
retrieving revision 1.40
diff -u -p -r1.40 if_switch.c
--- sys/net/if_switch.c 19 Jan 2021 19:39:14 -0000 1.40
+++ sys/net/if_switch.c 19 Feb 2021 20:24:37 -0000
@@ -196,6 +196,7 @@ switch_clone_destroy(struct ifnet *ifp)
  struct switch_port *swpo, *tp;
  struct ifnet *ifs;
 
+ NET_LOCK();
  TAILQ_FOREACH_SAFE(swpo, &sc->sc_swpo_list, swpo_list_next, tp) {
  if ((ifs = if_get(swpo->swpo_ifindex)) != NULL) {
  switch_port_detach(ifs);
@@ -204,6 +205,7 @@ switch_clone_destroy(struct ifnet *ifp)
  log(LOG_ERR, "failed to cleanup on ifindex(%d)\n",
     swpo->swpo_ifindex);
  }
+ NET_UNLOCK();
  LIST_REMOVE(sc, sc_switch_next);
  bstp_destroy(sc->sc_stp);
  swofp_destroy(sc);

Reply | Threaded
Open this post in threaded view
|

Re: switch(4): fix netlock assertion within ifpromisc()

Hrvoje Popovski
On 19.2.2021. 21:50, Vitaliy Makkoveev wrote:

> As it was reported [1] switch(4) triggers NET_ASSERT_LOCKED() while
> we perform ifconfig(8) destroy. ifpromisc() requires netlock to be held.
> This is true while switch_port_detach() and underlay ifpromisc() called
> through switch_ioctl(). But while we destroy switch(4) interface we call
> ifpromisc() without netlock. We can't add netlock just around
> ifpromisc() because switch_port_detach() which calls it called by
> switch_ioctl() with netlock held and by switch_clone_destroy() without
> netlock held. So the solution is to add netlock to destroy path. Diff
> below adds it around the whole foreach loop where we call
> switch_port_detach().
>
> 1. https://marc.info/?l=openbsd-bugs&m=161338077403538&w=2

i'm confirming that there are no more logs ..


tnx ..