| Message ID | 20210222121017.933649049@linuxfoundation.org |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
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 <emmanuel.grumbach@intel.com> > > [ Upstream commit 98c7d21f957b10d9c07a3a60a3a5a8f326a197e5 ] > > I hit a NULL pointer exception in this function when the > init flow went really bad. > > Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com> > Signed-off-by: Luca Coelho <luciano.coelho@intel.com> > Signed-off-by: Kalle Valo <kvalo@codeaurora.org> > Link: https://lore.kernel.org/r/iwlwifi.20210115130252.2e8da9f2c132.I0234d4b8ddaf70aaa5028a20c863255e05bc1f84@changeid > Signed-off-by: Sasha Levin <sashal@kernel.org> > --- > 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. diff --git a/drivers/net/wireless/iwlwifi/pcie/tx.c b/drivers/net/wireless/iwlwifi/pcie/tx.c index cb03c2855019b7..7584796131fa41 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", > spin_lock_bh(&txq->lock); > while (q->write_ptr != q->read_ptr) { > IWL_DEBUG_TX_REPLY(trans, "Q %d Free %d\n", > -- > 2.27.0 > > Best regards, Nobuhiro
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 <emmanuel.grumbach@intel.com> > > > > [ Upstream commit 98c7d21f957b10d9c07a3a60a3a5a8f326a197e5 ] > > > > I hit a NULL pointer exception in this function when the > > init flow went really bad. > > > > Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com> > > Signed-off-by: Luca Coelho <luciano.coelho@intel.com> > > Signed-off-by: Kalle Valo <kvalo@codeaurora.org> > > Link: https://lore.kernel.org/r/iwlwifi.20210115130252.2e8da9f2c132.I0234d4b8ddaf70aaa5028a20c863255e05bc1f84@changeid > > Signed-off-by: Sasha Levin <sashal@kernel.org> > > --- > > 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. thanks, greg k-h
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 <emmanuel.grumbach@intel.com> > > > > > > [ Upstream commit 98c7d21f957b10d9c07a3a60a3a5a8f326a197e5 ] > > > > > > I hit a NULL pointer exception in this function when the > > > init flow went really bad. > > > > > > Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com> > > > Signed-off-by: Luca Coelho <luciano.coelho@intel.com> > > > Signed-off-by: Kalle Valo <kvalo@codeaurora.org> > > > Link: https://lore.kernel.org/r/iwlwifi.20210115130252.2e8da9f2c132.I0234d4b8ddaf70aaa5028a20c863255e05bc1f84@changeid > > > Signed-off-by: Sasha Levin <sashal@kernel.org> > > > --- > > > 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 From 85913eb9a7b61e2baae3818e69afe79b76e122d2 Mon Sep 17 00:00:00 2001 From: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp> 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) <nobuhiro1.iwamatsu@toshiba.co.jp> --- 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
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; + } + spin_lock_bh(&txq->lock); while (q->write_ptr != q->read_ptr) { IWL_DEBUG_TX_REPLY(trans, "Q %d Free %d\n",