From patchwork Fri Jul 17 07:13:05 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Fenkart X-Patchwork-Id: 6813451 X-Patchwork-Delegate: kvalo@adurom.com Return-Path: X-Original-To: patchwork-linux-wireless@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 2C8139F358 for ; Fri, 17 Jul 2015 07:13:51 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4F60E20653 for ; Fri, 17 Jul 2015 07:13:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 54D0D2065D for ; Fri, 17 Jul 2015 07:13:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757011AbbGQHNs (ORCPT ); Fri, 17 Jul 2015 03:13:48 -0400 Received: from mail-wi0-f175.google.com ([209.85.212.175]:35751 "EHLO mail-wi0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756995AbbGQHNr (ORCPT ); Fri, 17 Jul 2015 03:13:47 -0400 Received: by wibxm9 with SMTP id xm9so31466397wib.0 for ; Fri, 17 Jul 2015 00:13:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=YTCuWVgMXniOOtnVXWVwPwuWceleztNjEQACeoS9f50=; b=SRWxiQZzlBN0DkVVZ3BjLXjNUqoQhODZxhLTBUJmJ94BSNj/X3wwllg1FY5wsq1kC8 fXswt2ozeH/h0+jSxjWGPCk6Um2z0XhXXlhSBvG4YVVVb5vyo6kMxwhEcUnM0OjwOrtm int3Q8EzZSCxAWLFHGrYAaNPOpAZ9rmAo0YMIYEwT9/se4iQbTeO/w/c/VoaJXmVHnB7 WOI881GLa5Mmdo87KcKG+UWhL7uUDw4qwis+AFjuKvHp+UMgA/tFJ+A5YvX/ns3h1Y/G VVNWb38gBOcWHb1wP0raJvr8hc1uPM6EqC8f01WeM0edVfgdCHLANixPAy1QwHtK1zvT 1BdA== X-Received: by 10.181.11.229 with SMTP id el5mr14008992wid.40.1437117226033; Fri, 17 Jul 2015 00:13:46 -0700 (PDT) Received: from localhost (ip-89-176-167-254.net.upcbroadband.cz. [89.176.167.254]) by smtp.gmail.com with ESMTPSA id l14sm16899680wjq.21.2015.07.17.00.13.44 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 17 Jul 2015 00:13:45 -0700 (PDT) From: Andreas Fenkart To: linux-wireless@vger.kernel.org Cc: Amitkumar Karwar , Kalle Valo , Andreas Fenkart Subject: [PATCH 3/4] mwifiex: remove CMD_F_CANCELED flag Date: Fri, 17 Jul 2015 09:13:05 +0200 Message-Id: <1437117186-25243-4-git-send-email-afenkart@gmail.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1437117186-25243-1-git-send-email-afenkart@gmail.com> References: <1437117186-25243-1-git-send-email-afenkart@gmail.com> Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Spam-Status: No, score=-8.0 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 Acked-by: Amitkumar Karwar --- 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