On Thu, Jun 29, 2023 at 12:07:51PM +0200, Stefan Sperling wrote:
> On Thu, Jun 29, 2023 at 11:31:33AM +0200, Martin Pieuchot wrote:
> > On 29/06/23(Thu) 11:17, Stefan Sperling wrote:
> > > 
> > > iwm_intr already runs at IPL_NET. What else would be required?
> > 
> > Are we sure all accesses to `ic_tree' are run under KERNEL_LOCK()+splnet()?
> 
> A quick look through net80211 doesn't suggest any problem here.
> 
> I assume the KERNEL_LOCK is implied since interrupt handlers in wireless
> drivers should not be marked MPSAFE. Userland has an ioctl to query the
> nodes tree but cannot modify it. New nodes are only added during scans n
> interrupt context under SPLNET. (Ignoring the timeouts in hostap mode for
> now, since this crash is in iwm which does not support hostap mode).
> 
> Nodes can be removed in ieee80211_free_allnodes() via iwm_newstate_task()
> and sc->sc_newstate, which runs in a dedicated task queue which is not
> marked MPSAFE and thus runs with the kernel lock held.
> ieee80211_free_allnodes() raises to IPL_NET before modyfing the tree.
> 
> While looking at this I spotted an unrelated problem: iwm/iwx add and delete
> bgscan_done_task via different task queues. This means bgscan_done_task will
> not be cancelled properly in iwm_newstate(). This bug originated in a commit
> I made in on December 2 2021 and hasn't been noticed yet.
> 
> I would just move this task to the systq, where it gets deleted from. ok?
> 

ok mvs

> diff /usr/src
> commit - 0c5a9349528207d3a937cab66be92baf3c29da40
> path + /usr/src
> blob - 1547120ea8f2bc861af585999863012e6ce6655c
> file + sys/dev/pci/if_iwm.c
> --- sys/dev/pci/if_iwm.c
> +++ sys/dev/pci/if_iwm.c
> @@ -8574,7 +8574,7 @@ iwm_bgscan_done(struct ieee80211com *ic,
>       free(sc->bgscan_unref_arg, M_DEVBUF, sc->bgscan_unref_arg_size);
>       sc->bgscan_unref_arg = arg;
>       sc->bgscan_unref_arg_size = arg_size;
> -     iwm_add_task(sc, sc->sc_nswq, &sc->bgscan_done_task);
> +     iwm_add_task(sc, systq, &sc->bgscan_done_task);
>  }
>  
>  void
> blob - 8aa8740bcf864ee470b7284da02d0c862e2bee99
> file + sys/dev/pci/if_iwx.c
> --- sys/dev/pci/if_iwx.c
> +++ sys/dev/pci/if_iwx.c
> @@ -7607,7 +7607,7 @@ iwx_bgscan_done(struct ieee80211com *ic,
>       free(sc->bgscan_unref_arg, M_DEVBUF, sc->bgscan_unref_arg_size);
>       sc->bgscan_unref_arg = arg;
>       sc->bgscan_unref_arg_size = arg_size;
> -     iwx_add_task(sc, sc->sc_nswq, &sc->bgscan_done_task);
> +     iwx_add_task(sc, systq, &sc->bgscan_done_task);
>  }
>  
>  void
> 

Reply via email to