Hi Andreas, > From: Andreas Fenkart [mailto:afenkart@xxxxxxxxx] > Sent: Friday, July 17, 2015 12:43 PM > To: linux-wireless@xxxxxxxxxxxxxxx > Cc: Amitkumar Karwar; Kalle Valo; Andreas Fenkart > Subject: [PATCH 3/4] mwifiex: remove CMD_F_CANCELED flag > > CMD_F_CANCELED was used to abort mwifiex_process_cmdresp in case it > already started or starts processing the cmd. > But this was probably not working the way intended: > - it is racy: mwifiex_process_cmdresp might already have passed that > test and is continuing to use the cmd node being recycled > - mwifiex_process_cmdresp repeatedly uses adapter->curr_cmd which > we just set to NULL > - mwifiex_recycle_cmd_node will clear the flag > > The reason why it probably works is that mwifiex_cancel_pending_ioctl is > only called from mwifiex_cmd_timeout_func, where the there is little > chance of a command response still arriving > > Signed-off-by: Andreas Fenkart <afenkart@xxxxxxxxx> > --- > drivers/net/wireless/mwifiex/cmdevt.c | 23 ++++++++++------------- > drivers/net/wireless/mwifiex/fw.h | 1 - > 2 files changed, 10 insertions(+), 14 deletions(-) > > diff --git a/drivers/net/wireless/mwifiex/cmdevt.c > b/drivers/net/wireless/mwifiex/cmdevt.c > index 87b6dee..6458e17 100644 > --- a/drivers/net/wireless/mwifiex/cmdevt.c > +++ b/drivers/net/wireless/mwifiex/cmdevt.c > @@ -807,17 +807,6 @@ int mwifiex_process_cmdresp(struct mwifiex_adapter > *adapter) > adapter->is_cmd_timedout = 0; > > resp = (struct host_cmd_ds_command *) adapter->curr_cmd->resp_skb- > >data; > - if (adapter->curr_cmd->cmd_flag & CMD_F_CANCELED) { > - mwifiex_dbg(adapter, ERROR, > - "CMD_RESP: %#x been canceled\n", > - le16_to_cpu(resp->command)); > - mwifiex_recycle_cmd_node(adapter, adapter->curr_cmd); > - spin_lock_irqsave(&adapter->mwifiex_cmd_lock, flags); > - adapter->curr_cmd = NULL; > - spin_unlock_irqrestore(&adapter->mwifiex_cmd_lock, flags); > - return -1; > - } > - > if (adapter->curr_cmd->cmd_flag & CMD_F_HOSTCMD) { > /* Copy original response back to response buffer */ > struct mwifiex_ds_misc_cmd *hostcmd; > @@ -1090,10 +1079,18 @@ mwifiex_cancel_pending_ioctl(struct > mwifiex_adapter *adapter) > (adapter->curr_cmd->wait_q_enabled)) { > spin_lock_irqsave(&adapter->mwifiex_cmd_lock, cmd_flags); > cmd_node = adapter->curr_cmd; > - cmd_node->cmd_flag |= CMD_F_CANCELED; > - mwifiex_recycle_cmd_node(adapter, cmd_node); > + /* setting curr_cmd to NULL is quite dangerous, because > + * mwifiex_process_cmdresp checks curr_cmd to be != NULL > + * at the beginning then relies on it and dereferences > + * it at will > + * this probably works since mwifiex_cmd_timeout_func > + * is the only caller of this function and responses > + * at that point > + */ > adapter->curr_cmd = NULL; > spin_unlock_irqrestore(&adapter->mwifiex_cmd_lock, > cmd_flags); > + > + mwifiex_recycle_cmd_node(adapter, cmd_node); > } > > /* Cancel all pending scan command */ > diff --git a/drivers/net/wireless/mwifiex/fw.h > b/drivers/net/wireless/mwifiex/fw.h > index cd09051..c71e6b2 100644 > --- a/drivers/net/wireless/mwifiex/fw.h > +++ b/drivers/net/wireless/mwifiex/fw.h > @@ -432,7 +432,6 @@ enum P2P_MODES { > > > #define CMD_F_HOSTCMD (1 << 0) > -#define CMD_F_CANCELED (1 << 1) > > #define HostCmd_CMD_ID_MASK 0x0fff > > -- > 2.1.4 Acked-by: Amitkumar Karwar <akarwar@xxxxxxxxxxx> Regards, Amitkumar -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
