Re: [PATCH v2] mwifiex: fix kernel crash after shutdown command timeout

[Dmitry Torokhov]

On Thu, Mar 16, 2017 at 03:58:52PM +0530, Amitkumar Karwar wrote:
> We observed a SHUTDOWN command timeout during reboot stress test
> due to a corner case firmware bug. It leads to use-after-free on
> adapter structure pointer and crash.
> 
> Let's add MWIFIEX_IFACE_WORK_DONT_RUN work flag to avoid executing
> any work scheduled after cancel_work_sync() call in teardown path
> to resolve the issue.
> 
> Signed-off-by: Amitkumar Karwar <akar...@marvell.com>
> ---
> v2: New work_flag has been added to resolve the issue cleanly as per
> Brian's suggestion.
> ---
>  drivers/net/wireless/marvell/mwifiex/main.h | 1 +
>  drivers/net/wireless/marvell/mwifiex/pcie.c | 4 ++++
>  drivers/net/wireless/marvell/mwifiex/sdio.c | 4 ++++
>  3 files changed, 9 insertions(+)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/main.h 
> b/drivers/net/wireless/marvell/mwifiex/main.h
> index 5c82972..d5b1fd6 100644
> --- a/drivers/net/wireless/marvell/mwifiex/main.h
> +++ b/drivers/net/wireless/marvell/mwifiex/main.h
> @@ -510,6 +510,7 @@ struct mwifiex_roc_cfg {
>  enum mwifiex_iface_work_flags {
>       MWIFIEX_IFACE_WORK_DEVICE_DUMP,
>       MWIFIEX_IFACE_WORK_CARD_RESET,
> +     MWIFIEX_IFACE_WORK_DONT_RUN,
>  };
>  
>  struct mwifiex_private {
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c 
> b/drivers/net/wireless/marvell/mwifiex/pcie.c
> index a0d9180..bb3d798 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> @@ -294,6 +294,7 @@ static void mwifiex_pcie_remove(struct pci_dev *pdev)
>       if (!adapter || !adapter->priv_num)
>               return;
>  
> +     set_bit(MWIFIEX_IFACE_WORK_DONT_RUN, &card->work_flags);
>       cancel_work_sync(&card->work);
>  
>       reg = card->pcie.reg;
> @@ -2721,6 +2722,9 @@ static void mwifiex_pcie_work(struct work_struct *work)
>       struct pcie_service_card *card =
>               container_of(work, struct pcie_service_card, work);
>  
> +     if (test_bit(MWIFIEX_IFACE_WORK_DONT_RUN, &card->work_flags))
> +             return;

I do not see how this could possible prevent use-after-free, assuming
that the "card" memory is gone by the time mwifiex_pcie_work() gets to
run. You need to check this flag before queueing firmware dump work, and
make sure it is not racy with setting this flag in mwifiex_pcie_remove()
(and sdio).

Thanks.

-- 
Dmitry