2021-02-22 13:30:03

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH 4.4 04/35] iwlwifi: pcie: add a NULL check in iwl_pcie_txq_unmap

From: Emmanuel Grumbach <[email protected]>

[ Upstream commit 98c7d21f957b10d9c07a3a60a3a5a8f326a197e5 ]

I hit a NULL pointer exception in this function when the
init flow went really bad.

Signed-off-by: Emmanuel Grumbach <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
Signed-off-by: Kalle Valo <[email protected]>
Link: https://lore.kernel.org/r/iwlwifi.20210115130252.2e8da9f2c132.I0234d4b8ddaf70aaa5028a20c863255e05bc1f84@changeid
Signed-off-by: Sasha Levin <[email protected]>
---
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;
+ }
+
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




2021-02-25 06:58:40

by Nobuhiro Iwamatsu

[permalink] [raw]
Subject: Re: [PATCH 4.4 04/35] iwlwifi: pcie: add a NULL check in iwl_pcie_txq_unmap

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 <[email protected]>
>
> [ Upstream commit 98c7d21f957b10d9c07a3a60a3a5a8f326a197e5 ]
>
> I hit a NULL pointer exception in this function when the
> init flow went really bad.
>
> Signed-off-by: Emmanuel Grumbach <[email protected]>
> Signed-off-by: Luca Coelho <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>
> Link: https://lore.kernel.org/r/iwlwifi.20210115130252.2e8da9f2c132.I0234d4b8ddaf70aaa5028a20c863255e05bc1f84@changeid
> Signed-off-by: Sasha Levin <[email protected]>
> ---
> 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

2021-02-25 09:56:40

by Nobuhiro Iwamatsu

[permalink] [raw]
Subject: Re: [PATCH 4.4 04/35] iwlwifi: pcie: add a NULL check in iwl_pcie_txq_unmap

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 <[email protected]>
> > >
> > > [ Upstream commit 98c7d21f957b10d9c07a3a60a3a5a8f326a197e5 ]
> > >
> > > I hit a NULL pointer exception in this function when the
> > > init flow went really bad.
> > >
> > > Signed-off-by: Emmanuel Grumbach <[email protected]>
> > > Signed-off-by: Luca Coelho <[email protected]>
> > > Signed-off-by: Kalle Valo <[email protected]>
> > > Link: https://lore.kernel.org/r/iwlwifi.20210115130252.2e8da9f2c132.I0234d4b8ddaf70aaa5028a20c863255e05bc1f84@changeid
> > > Signed-off-by: Sasha Levin <[email protected]>
> > > ---
> > > 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


Attachments:
(No filename) (2.25 kB)
0001-iwlwifi-pcie-fix-to-correct-null-check.patch (1.56 kB)
Download all attachments

2021-02-25 11:44:15

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 4.4 04/35] iwlwifi: pcie: add a NULL check in iwl_pcie_txq_unmap

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 <[email protected]>
> >
> > [ Upstream commit 98c7d21f957b10d9c07a3a60a3a5a8f326a197e5 ]
> >
> > I hit a NULL pointer exception in this function when the
> > init flow went really bad.
> >
> > Signed-off-by: Emmanuel Grumbach <[email protected]>
> > Signed-off-by: Luca Coelho <[email protected]>
> > Signed-off-by: Kalle Valo <[email protected]>
> > Link: https://lore.kernel.org/r/iwlwifi.20210115130252.2e8da9f2c132.I0234d4b8ddaf70aaa5028a20c863255e05bc1f84@changeid
> > Signed-off-by: Sasha Levin <[email protected]>
> > ---
> > 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