Hi Kalle, On 5/5/23 08:47, Kalle Valo wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > <Ajay.Kathat@xxxxxxxxxxxxx> writes: > >> Fix for kernel crash observed with following test procedure [1]: >> while true; >> do ifconfig wlan0 up; >> iw dev wlan0 scan & >> ifconfig wlan0 down; >> done >> >> During the above test procedure, the scan results are received from firmware >> for 'iw scan' command gets queued even when the interface is going down. It >> was causing the kernel oops when dereferencing the freed pointers. >> >> For synchronization, 'mac_close()' calls flush_workqueue() to block its >> execution till all pending work is completed. Afterwards 'wilc->close' flag >> which is set before the flush_workqueue() should avoid adding new work. >> Added 'wilc->close' check in wilc_handle_isr() which is common for >> SPI/SDIO bus to ignore the interrupts from firmware that inturns adds the >> work since the interface is getting closed. >> >> 1. https://lore.kernel.org/linux-wireless/20221024135407.7udo3dwl3mqyv2yj@xxxxxxxxxxxx/ >> >> Reported-by: Michael Walle <mwalle@xxxxxxxxxx> >> Signed-off-by: Ajay Singh <ajay.kathat@xxxxxxxxxxxxx> > > [...] > >> @@ -781,13 +776,15 @@ static int wilc_mac_close(struct net_device *ndev) >> if (vif->ndev) { >> netif_stop_queue(vif->ndev); >> >> + if (wl->open_ifcs == 0) >> + wl->close = 1; >> + > > wl-close is an int, I wonder if it's racy to int as a flag like this? In > cases like this I usually use set_bit() & co because those guarantee > atomicity, though don't know if that's overkill. > I think it's a good idea to use an atomic operation but I am not sure if using atomic for 'wl->close' will have much impact. For instance, if any new work gets added to the workqueue before the 'wl->close=1' is fully completed, then that work would get executed as normal. However, I feel it's safe to define 'wl->close' as atomic_t type. I will prepare the conversion patch and will try to include it along with the updated version of this patch. Regards, Ajay
