From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 52855C433E6 for ; Thu, 25 Feb 2021 08:50:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2287664EDD for ; Thu, 25 Feb 2021 08:50:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236007AbhBYItv (ORCPT ); Thu, 25 Feb 2021 03:49:51 -0500 Received: from mo-csw1116.securemx.jp ([210.130.202.158]:60952 "EHLO mo-csw.securemx.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232177AbhBYItj (ORCPT ); Thu, 25 Feb 2021 03:49:39 -0500 Received: by mo-csw.securemx.jp (mx-mo-csw1116) id 11P8lG6L026829; Thu, 25 Feb 2021 17:47:16 +0900 X-Iguazu-Qid: 2wGra9XxFUulKTwMxY X-Iguazu-QSIG: v=2; s=0; t=1614242836; q=2wGra9XxFUulKTwMxY; m=3LZse+GcVvgYq21VFTdnN2QhZC92Dp5MRMb2uEy9Y9k= Received: from imx2-a.toshiba.co.jp (imx2-a.toshiba.co.jp [106.186.93.35]) by relay.securemx.jp (mx-mr1113) id 11P8lF6k012947 (version=TLSv1.2 cipher=AES128-GCM-SHA256 bits=128 verify=NOT); Thu, 25 Feb 2021 17:47:15 +0900 Received: from enc01.toshiba.co.jp (enc01.toshiba.co.jp [106.186.93.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by imx2-a.toshiba.co.jp (Postfix) with ESMTPS id 6D1BD1000DD; Thu, 25 Feb 2021 17:47:15 +0900 (JST) Received: from hop001.toshiba.co.jp ([133.199.164.63]) by enc01.toshiba.co.jp with ESMTP id 11P8lE3N005672; Thu, 25 Feb 2021 17:47:15 +0900 Date: Thu, 25 Feb 2021 17:47:13 +0900 From: Nobuhiro Iwamatsu To: Greg Kroah-Hartman Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org, Emmanuel Grumbach , Luca Coelho , Kalle Valo , Sasha Levin Subject: Re: [PATCH 4.4 04/35] iwlwifi: pcie: add a NULL check in iwl_pcie_txq_unmap X-TSB-HOP: ON Message-ID: <20210225084713.msqf5bqh7j42l6jm@toshiba.co.jp> References: <20210222121013.581198717@linuxfoundation.org> <20210222121017.933649049@linuxfoundation.org> <20210225060446.auoymjxg5cuzlism@toshiba.co.jp> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="mret2h7hhsygveq2" Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --mret2h7hhsygveq2 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi, On Thu, Feb 25, 2021 at 09:14:42AM +0100, Greg Kroah-Hartman wrote: > On Thu, Feb 25, 2021 at 03:04:46PM +0900, Nobuhiro Iwamatsu wrote: > > Hi, > > > > Sorry for the report after the release. > > > > On Mon, Feb 22, 2021 at 01:36:00PM +0100, Greg Kroah-Hartman wrote: > > > From: Emmanuel Grumbach > > > > > > [ Upstream commit 98c7d21f957b10d9c07a3a60a3a5a8f326a197e5 ] > > > > > > I hit a NULL pointer exception in this function when the > > > init flow went really bad. > > > > > > Signed-off-by: Emmanuel Grumbach > > > Signed-off-by: Luca Coelho > > > Signed-off-by: Kalle Valo > > > Link: https://lore.kernel.org/r/iwlwifi.20210115130252.2e8da9f2c132.I0234d4b8ddaf70aaa5028a20c863255e05bc1f84@changeid > > > Signed-off-by: Sasha Levin > > > --- > > > drivers/net/wireless/iwlwifi/pcie/tx.c | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/drivers/net/wireless/iwlwifi/pcie/tx.c b/drivers/net/wireless/iwlwifi/pcie/tx.c > > > index 8dfe6b2bc7031..cb03c2855019b 100644 > > > --- a/drivers/net/wireless/iwlwifi/pcie/tx.c > > > +++ b/drivers/net/wireless/iwlwifi/pcie/tx.c > > > @@ -585,6 +585,11 @@ static void iwl_pcie_txq_unmap(struct iwl_trans *trans, int txq_id) > > > struct iwl_txq *txq = &trans_pcie->txq[txq_id]; > > > struct iwl_queue *q = &txq->q; > > > > > > + if (!txq) { > > > + IWL_ERR(trans, "Trying to free a queue that wasn't allocated?\n"); > > > + return; > > > + } > > > + > > > > I think that this fix is not enough. > > If txq is NULL, an error will occur with "struct iwl_queue * q = & txq->q;". > > The following changes are required. > > Is this a 4.4-only thing, or is this issue also in Linus's tree as well? > If Linus's tree, please submit this as a normal patch so we can apply it > there first. I did not have enough explanation. This issue is only 4.4.y tree. The same patch has been applied to the other trees, but with the correct fixes. Also this issue is not in Linus's tree. This is due to incorrect fixes in this commit. I attached a patch for this issue. > > thanks, > > greg k-h > Best regards, Nobuhiro --mret2h7hhsygveq2 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0001-iwlwifi-pcie-fix-to-correct-null-check.patch" >From 85913eb9a7b61e2baae3818e69afe79b76e122d2 Mon Sep 17 00:00:00 2001 From: Nobuhiro Iwamatsu Date: Thu, 25 Feb 2021 17:30:47 +0900 Subject: [PATCH] iwlwifi: pcie: fix to correct null check The fixes made in commit: 4ae5798004d8 ("iwlwifi: pcie: add a NULL check in iwl_pcie_txq_unmap") is not enough. This still have problems with null references. This provides the correct fix. Also, this is a problem only in 4.4.y. This patch has been applied to other LTS trees, but with the correct fixes. Fixes: 4ae5798004d8 ("iwlwifi: pcie: add a NULL check in iwl_pcie_txq_unmap") Cc: stable@vger.kernel.org Signed-off-by: Nobuhiro Iwamatsu (CIP) --- drivers/net/wireless/iwlwifi/pcie/tx.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/iwlwifi/pcie/tx.c b/drivers/net/wireless/iwlwifi/pcie/tx.c index cb03c2855019..7584796131fa 100644 --- a/drivers/net/wireless/iwlwifi/pcie/tx.c +++ b/drivers/net/wireless/iwlwifi/pcie/tx.c @@ -583,13 +583,15 @@ static void iwl_pcie_txq_unmap(struct iwl_trans *trans, int txq_id) { struct iwl_trans_pcie *trans_pcie = IWL_TRANS_GET_PCIE_TRANS(trans); struct iwl_txq *txq = &trans_pcie->txq[txq_id]; - struct iwl_queue *q = &txq->q; + struct iwl_queue *q; if (!txq) { IWL_ERR(trans, "Trying to free a queue that wasn't allocated?\n"); return; } + q = &txq->q; + spin_lock_bh(&txq->lock); while (q->write_ptr != q->read_ptr) { IWL_DEBUG_TX_REPLY(trans, "Q %d Free %d\n", -- 2.30.0.rc2 --mret2h7hhsygveq2--