linux-kernel - Re: [REVERT] be6b38bcb175613f239e0b302607db346472c6b6. v2.6.34-rc3-406 oops with 4965AGN wireless

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.64.1004090251210.11582@boston.corp.fedex.com>
Date:	Fri, 9 Apr 2010 03:27:53 +0800 (SGT)
From:	Jeff Chua <jeff.chua.linux@...il.com>
To:	Wey-Yi Guy <wey-yi.w.guy@...el.com>,
	Shanyu Zhao <shanyu.zhao@...el.com>,
	Reinette Chatre <reinette.chatre@...el.com>, stable@...nel.org,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [REVERT] be6b38bcb175613f239e0b302607db346472c6b6. v2.6.34-rc3-406
 oops with 4965AGN wireless



On Fri, Apr 9, 2010 at 1:19 AM, Linus Torvalds 
<torvalds@...ux-foundation.org> wrote:

> I don't mind reverting patches, but I hate doing it blind and without 
> _any_ chance for the guilty party to try to fix it first.

> Can you post the oops that your subject implies happened? Maybe the 
> driver authors can find the proper fix without a revert?

The screen is so fast and never stops so I don't know how to capture that. 
I just tried the patch from Wey and had to modify it slightly to make it 
work.



On Fri, Apr 9, 2010 at 2:50 AM, Guy, Wey-Yi <wey-yi.w.guy@...el.com> 
wrote:

> Sorry for the problem caused by this patch, it is my mistake using the
similar approach for 4965 like the newer devices. could you try the
attach patch to see if this fix your system freeze problem.

Wey, the patch doesn't apply cleanly. I believe you meant "iwl_" instead 
of "iwlagn_".

Also, the checking for IWL_INVALID_STATION should be done after the "} 
else {" as in the original code prior to your patch. I just verified this 
with the patch below and it no longer oops.



On Fri, Apr 9, 2010 at 2:13 AM, Al Viro <viro@...iv.linux.org.uk> wrote:

> So what happens if we hit sta_id == IWL_INVALID_STATION and
> !txq->sched_retry?

> I'm not familiar with that code and I don't have the hardware, so this 
> is just from RTFS, but... might make sense to replace that call of 
> iwl_free_tfds_in_queue with
>       if (sta_id == IWL_INVALID_STATION)
>              printk(KERN_ERR "buggered");
>       else
>              iwl_free_tfds_in_queue(priv, sta_id, tid, freed);


I just tried that, but not seeing any invalid station messages. The 
KERN_ERR has been added below.




On Fri, Apr 9, 2010 at 4:09 AM, Guy, Wey-Yi <wey-yi.w.guy@...el.com> 
wrote:

> We can check for IWL_INVALID_STATION and print log, but need to check
> for qc to make sure it is valid; maybe something like


Ok, added below.



Thanks,
Jeff

--- drivers/net/wireless/iwlwifi/iwl-4965.c.org	2010-04-09 02:11:45.000000000 +0800
+++ drivers/net/wireless/iwlwifi/iwl-4965.c	2010-04-09 03:21:57.000000000 +0800
@@ -2012,10 +2012,18 @@

  		if (txq->q.read_ptr != (scd_ssn & 0xff)) {
  			index = iwl_queue_dec_wrap(scd_ssn & 0xff, txq->q.n_bd);
-			IWL_DEBUG_TX_REPLY(priv, "Retry scheduler reclaim scd_ssn "
-					   "%d index %d\n", scd_ssn , index);
+			IWL_DEBUG_TX_REPLY(priv,
+					"Retry scheduler reclaim scd_ssn "
+					"%d index %d\n", scd_ssn , index);
  			freed = iwl_tx_queue_reclaim(priv, txq_id, index);
  			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+			if (qc && likely(sta_id != IWL_INVALID_STATION))
+				iwl_free_tfds_in_queue(priv, sta_id, tid,
+					freed);
+			else {
+				if (sta_id == IWL_INVALID_STATION)
+				       IWL_ERR(priv, "invalid station");
+			}

  			if (priv->mac80211_registered &&
  			    (iwl_queue_space(&txq->q) > txq->q.low_mark) &&
@@ -2041,14 +2049,20 @@
  				   tx_resp->failure_frame);

  		freed = iwl_tx_queue_reclaim(priv, txq_id, index);
-		iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+		if (qc && likely(sta_id != IWL_INVALID_STATION))
+			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+		else {
+			if (sta_id == IWL_INVALID_STATION)
+			       IWL_ERR(priv, "invalid station");
+		}

  		if (priv->mac80211_registered &&
  		    (iwl_queue_space(&txq->q) > txq->q.low_mark))
  			iwl_wake_queue(priv, txq_id);
  	}

-	iwl_txq_check_empty(priv, sta_id, tid, txq_id);
+	if (qc && likely(sta_id != IWL_INVALID_STATION))
+		iwl_txq_check_empty(priv, sta_id, tid, txq_id);

  	if (iwl_check_bits(status, TX_ABORT_REQUIRED_MSK))
  		IWL_ERR(priv, "TODO:  Implement Tx ABORT REQUIRED!!!\n");
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/