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

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

[Stanislaw Gruszka]

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 <sgruszka@redhat.com>
---
 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

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

[reinette chatre]

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 <sgruszka@redhat.com>
> ---

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

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

[John W. Linville]

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 <sgruszka@redhat.com>
> > ---
> 
> 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
linville@tuxdriver.com			might be all we have.  Be ready.