Hi Ganapathi,

On Fri, Jun 01, 2018 at 04:11:20PM +0530, Ganapathi Bhat wrote:
> Race condition is observed during rmmod of mwifiex_usb:
> 
> 1. The rmmod thread will call mwifiex_usb_disconnect(), download
>    SHUTDOWN command and do wait_event_interruptible_timeout(),
>    waiting for response.
> 
> 2. The main thread will handle the response and will do a
>    wake_up_interruptible(), unblocking rmmod thread.
> 
> 3. On getting unblocked, rmmod thread  will make rx_cmd.urb = NULL in
>    mwifiex_usb_free().
> 
> 4. The main thread will try to resubmit rx_cmd.urb in
>    mwifiex_usb_submit_rx_urb(), which is NULL.
> 
> To fix this, move mwifiex_usb_free() from mwifiex_usb_disconnect
> to mwifiex_unregister_dev(). Function mwifiex_unregister_dev() is
> called after flushing the command and RX work queues.
> 
> Signed-off-by: Brian Norris <[email protected]>

^^ I'm not sure if that line is quite accurate. While I nearly spelled
out what the patch would look like, you wrote it.

Anyway, patch seems good to me, assuming it tests out OK for you:

Reviewed-by: Brian Norris <[email protected]>

and if Kalle hasn't applied this yet, an alternative to Signed-off-by:

Suggested-by: Brian Norris <[email protected]>

> Signed-off-by: Ganapathi Bhat <[email protected]>
> ---
>  drivers/net/wireless/marvell/mwifiex/usb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c 
> b/drivers/net/wireless/marvell/mwifiex/usb.c
> index bc475b8..88f4c89 100644
> --- a/drivers/net/wireless/marvell/mwifiex/usb.c
> +++ b/drivers/net/wireless/marvell/mwifiex/usb.c
> @@ -644,8 +644,6 @@ static void mwifiex_usb_disconnect(struct usb_interface 
> *intf)
>                                        MWIFIEX_FUNC_SHUTDOWN);
>       }
>  
> -     mwifiex_usb_free(card);
> -
>       mwifiex_dbg(adapter, FATAL,
>                   "%s: removing card\n", __func__);
>       mwifiex_remove_card(adapter);
> @@ -1353,6 +1351,8 @@ static void mwifiex_unregister_dev(struct 
> mwifiex_adapter *adapter)
>  {
>       struct usb_card_rec *card = (struct usb_card_rec *)adapter->card;
>  
> +     mwifiex_usb_free(card);
> +
>       mwifiex_usb_cleanup_tx_aggr(adapter);
>  
>       card->adapter = NULL;
> -- 
> 1.9.1
> 

Reply via email to