2009-09-23 08:51:51

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH] iwlagn: fix panic in iwl{5000,4965}_rx_reply_tx

In some cases firmware can give us bad value of index in transmit
buffers array. This patch add sanity check for such values and return
from processing function instantly when it happens.

https://bugzilla.redhat.com/show_bug.cgi?id=521931

Patch was tested by reporter on iwl5000. I think check can be also
helpful for 4965.

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
drivers/net/wireless/iwlwifi/iwl-4965.c | 6 ++++++
drivers/net/wireless/iwlwifi/iwl-5000.c | 6 ++++++
2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-4965.c b/drivers/net/wireless/iwlwifi/iwl-4965.c
index 8f3d4bc..573818f 100644
--- a/drivers/net/wireless/iwlwifi/iwl-4965.c
+++ b/drivers/net/wireless/iwlwifi/iwl-4965.c
@@ -2019,6 +2019,12 @@ static int iwl4965_tx_status_reply_tx(struct iwl_priv *priv,
agg->frame_count, txq_id, idx);

hdr = iwl_tx_queue_get_hdr(priv, txq_id, idx);
+ if (!hdr) {
+ IWL_ERR(priv,
+ "BUG_ON idx doesn't point to valid skb"
+ " idx=%d, txq_id=%d\n", idx, txq_id);
+ return -1;
+ }

sc = le16_to_cpu(hdr->seq_ctrl);
if (idx != (SEQ_TO_SN(sc) & 0xff)) {
diff --git a/drivers/net/wireless/iwlwifi/iwl-5000.c b/drivers/net/wireless/iwlwifi/iwl-5000.c
index b3c648c..460f1fb 100644
--- a/drivers/net/wireless/iwlwifi/iwl-5000.c
+++ b/drivers/net/wireless/iwlwifi/iwl-5000.c
@@ -1139,6 +1139,12 @@ static int iwl5000_tx_status_reply_tx(struct iwl_priv *priv,
agg->frame_count, txq_id, idx);

hdr = iwl_tx_queue_get_hdr(priv, txq_id, idx);
+ if (!hdr) {
+ IWL_ERR(priv,
+ "BUG_ON idx doesn't point to valid skb"
+ " idx=%d, txq_id=%d\n", idx, txq_id);
+ return -1;
+ }

sc = le16_to_cpu(hdr->seq_ctrl);
if (idx != (SEQ_TO_SN(sc) & 0xff)) {
--
1.6.2.5



2009-09-24 21:38:06

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH] iwlagn: fix panic in iwl{5000,4965}_rx_reply_tx

Hi Stanislaw,

On Wed, 2009-09-23 at 01:51 -0700, Stanislaw Gruszka wrote:
> In some cases firmware can give us bad value of index in transmit
> buffers array. This patch add sanity check for such values and return
> from processing function instantly when it happens.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=521931
>
> Patch was tested by reporter on iwl5000. I think check can be also
> helpful for 4965.
>
> Signed-off-by: Stanislaw Gruszka <[email protected]>
> ---

I looked at the bugzilla entry and I think that there may be another fix
required here. After the driver submitted the five frames it received a
surprisingly large number of tx responses from firmware, with one of
these causing the problem. The bad value from the firmware may be a
result of something else done incorrectly by driver here since the
firmware has been trying for more than 40 times at this point to inform
driver about tx results.

I commented in that bugzilla and we can continue to debug this issue
there. Until then I'd like to hold off on this patch.

Reinette



2009-09-28 20:15:51

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH] iwlagn: fix panic in iwl{5000,4965}_rx_reply_tx

On Thu, Sep 24, 2009 at 02:38:07PM -0700, reinette chatre wrote:
> Hi Stanislaw,
>
> On Wed, 2009-09-23 at 01:51 -0700, Stanislaw Gruszka wrote:
> > In some cases firmware can give us bad value of index in transmit
> > buffers array. This patch add sanity check for such values and return
> > from processing function instantly when it happens.
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=521931
> >
> > Patch was tested by reporter on iwl5000. I think check can be also
> > helpful for 4965.
> >
> > Signed-off-by: Stanislaw Gruszka <[email protected]>
> > ---
>
> I looked at the bugzilla entry and I think that there may be another fix
> required here. After the driver submitted the five frames it received a
> surprisingly large number of tx responses from firmware, with one of
> these causing the problem. The bad value from the firmware may be a
> result of something else done incorrectly by driver here since the
> firmware has been trying for more than 40 times at this point to inform
> driver about tx results.
>
> I commented in that bugzilla and we can continue to debug this issue
> there. Until then I'd like to hold off on this patch.

Hmmm...well, I already sent it to Dave/Linus -- it's in 2.6.32-rc1...

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.