From patchwork Fri May 12 16:41:59 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Norris X-Patchwork-Id: 9724627 X-Patchwork-Delegate: kvalo@adurom.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 9F073601E7 for ; Fri, 12 May 2017 16:44:55 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8D41F28854 for ; Fri, 12 May 2017 16:44:55 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 81F6C2885E; Fri, 12 May 2017 16:44:55 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,RCVD_IN_DNSWL_HI,RCVD_IN_SORBS_SPAM autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 36B7028854 for ; Fri, 12 May 2017 16:44:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933224AbdELQm3 (ORCPT ); Fri, 12 May 2017 12:42:29 -0400 Received: from mail-pg0-f43.google.com ([74.125.83.43]:33744 "EHLO mail-pg0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933182AbdELQm1 (ORCPT ); Fri, 12 May 2017 12:42:27 -0400 Received: by mail-pg0-f43.google.com with SMTP id u187so32675385pgb.0 for ; Fri, 12 May 2017 09:42:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=oJhXwhEp+0Gs5xR/Iubf7hUPqudXgR2yXTrC1rD+Vxs=; b=EU01dbjtXGcZO6gF55poxIsHmxHZyZtOMhV57S0t2pMaaII/rKpT3yapBbnVysgwmS jwTju4xuK+GUqtkF50rntT2RiERE+vBfNkJw4g/GJj/EpSXGh+lI2e3JaTxVQcpzNL8y WfnTqj1rQuSXKdmDgjWIk1gq04uzn5/kDs8Zk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=oJhXwhEp+0Gs5xR/Iubf7hUPqudXgR2yXTrC1rD+Vxs=; b=J9LfZ25lsbgFb2Djy/2Ir8bYMe624VAGMQxT0lEVOlBgt8Hi2rDjb/Q2OB0s98FDQA F90DUAU/KTIk8ozEZ5Y/2RNCSfzeTmPzPPyk6TTXJj8uvHIRUm5zgeoEZh8qb4CA6gQN mE9sZoEI9zPKAo9Wv+r91y6Tqy2JjIYhlUrmBXKHDQ978e3ft4M1AmLHj5T+12Nsws81 3bojjz68nj9StLot+tWlt2hma0WTuEHXfN6+qc35pj7dUwSmNkHR7zTGkuGLBvkonFdk i4IDCrjd3PJaQfNttXqsbDVTL0LGkUckYqwAUpTzfmA660rF4LhFgBVIsIsVXK3trjiM Iybg== X-Gm-Message-State: AODbwcBeEDNos+w3Fbmr1IDsnn2rmWBdA33Fg8u5CFGRXblx/OrVcBjz uDsDFucGhMsI1CHJ0mWEuQ== X-Received: by 10.99.153.9 with SMTP id d9mr5394992pge.214.1494607346545; Fri, 12 May 2017 09:42:26 -0700 (PDT) Received: from ban.mtv.corp.google.com ([172.22.64.120]) by smtp.gmail.com with ESMTPSA id u74sm7304499pfk.58.2017.05.12.09.42.25 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 12 May 2017 09:42:26 -0700 (PDT) From: Brian Norris To: Ganapathi Bhat , Nishant Sarmukadam Cc: , Dmitry Torokhov , Amitkumar Karwar , Kalle Valo , linux-wireless@vger.kernel.org, Doug Anderson , Brian Norris Subject: [PATCH 02/11] mwifiex: Don't release tx_ba_stream_tbl_lock while iterating Date: Fri, 12 May 2017 09:41:59 -0700 Message-Id: <20170512164208.38725-2-briannorris@chromium.org> X-Mailer: git-send-email 2.13.0.rc2.291.g57267f2277-goog In-Reply-To: <20170512164208.38725-1-briannorris@chromium.org> References: <20170512164208.38725-1-briannorris@chromium.org> Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Douglas Anderson Despite the macro list_for_each_entry_safe() having the word "safe" in the name, it's still not actually safe to release the list spinlock while iterating over the list. The "safe" in the macro name actually only means that it's safe to delete the current entry while iterating over the list. Releasing the spinlock while iterating over the list means that someone else could come in and adjust the list while we don't have the spinlock. If they do that it can totally mix up our iteration and fully corrupt the list. Later iterating over a corrupted list while holding a spinlock and having IRQs off can cause all sorts of hard to debug problems. As evidenced by the other call to mwifiex_11n_delete_tx_ba_stream_tbl_entry() in mwifiex_11n_delete_all_tx_ba_stream_tbl(), it's actually safe to skip the spinlock release. Let's do that. No known problems are fixed by this patch, but it could fix all sorts of weird problems and it should be very safe. Signed-off-by: Douglas Anderson Signed-off-by: Brian Norris --- drivers/net/wireless/marvell/mwifiex/11n.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/net/wireless/marvell/mwifiex/11n.c b/drivers/net/wireless/marvell/mwifiex/11n.c index c174e79e6df2..c75b6abf16a0 100644 --- a/drivers/net/wireless/marvell/mwifiex/11n.c +++ b/drivers/net/wireless/marvell/mwifiex/11n.c @@ -764,14 +764,9 @@ void mwifiex_del_tx_ba_stream_tbl_by_ra(struct mwifiex_private *priv, u8 *ra) return; spin_lock_irqsave(&priv->tx_ba_stream_tbl_lock, flags); - list_for_each_entry_safe(tbl, tmp, &priv->tx_ba_stream_tbl_ptr, list) { - if (!memcmp(tbl->ra, ra, ETH_ALEN)) { - spin_unlock_irqrestore(&priv->tx_ba_stream_tbl_lock, - flags); + list_for_each_entry_safe(tbl, tmp, &priv->tx_ba_stream_tbl_ptr, list) + if (!memcmp(tbl->ra, ra, ETH_ALEN)) mwifiex_11n_delete_tx_ba_stream_tbl_entry(priv, tbl); - spin_lock_irqsave(&priv->tx_ba_stream_tbl_lock, flags); - } - } spin_unlock_irqrestore(&priv->tx_ba_stream_tbl_lock, flags); return;