netdev - Re: Fw: [PATCH] iwl3945: queue the right work if the scan needs to be aborted

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20101005085717.GA18012@redhat.com>
Date:	Tue, 5 Oct 2010 10:57:17 +0200
From:	Stanislaw Gruszka <sgruszka@...hat.com>
To:	Florian Mickler <florian@...kler.org>
Cc:	stable@...nel.org,
	"linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
	"Guy, Wey-Yi" <wey-yi.w.guy@...el.com>,
	"Chatre, Reinette" <reinette.chatre@...el.com>,
	Intel Linux Wireless <ilw@...ux.intel.com>,
	"John W. Linville" <linville@...driver.com>,
	"Berg, Johannes" <johannes.berg@...el.com>,
	"Cahill, Ben M" <ben.m.cahill@...el.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	linux-kernel@...r.kernel.org
Subject: Re: Fw: [PATCH] iwl3945: queue the right work if the scan needs to
 be aborted

On Tue, Oct 05, 2010 at 08:43:05AM +0200, Florian Mickler wrote:
> commit e7ee762cf074b0fd8eec483d0cef8fdbf0d04b81
> 
> 
> Begin forwarded message:
> On Fri, 2010-09-24 at 09:22 -0700, Florian Mickler wrote:
> > iwl3945's scan_completed calls into the mac80211 stack which triggers a
> > warn on if there is no scan outstanding.
> > 
> > This can be avoided by not calling scan_completed but abort_scan in
> > iwl3945_request_scan  in the done: branch of the function which is used
> > as an error out.
> > 
> > The done: branch seems to be an error-out branch, as, for example, if
> > iwl_is_ready(priv) returns false  the done: branch is executed.
> > 
> > NOTE:
> > I'm not familiar with the driver at all.
> > I just quickly scanned as a reaction to
> > 
> > https://bugzilla.kernel.org/show_bug.cgi?id=17722
> > 
> > the users of scan_completed in the  iwl3945 driver and noted the odd
> > discrepancy between the comment above this instance and the comment in
> > mac80211 scan_completed function.
> > Signed-off-by: Florian Mickler <florian@...kler.org>
> Acked-by: Wey-Yi Guy <wey-yi.w.guy@...el.com>
> > ---
> go into wireless-2.6 and stable only, scan fix already in
> wireless-next-2.6

> 
> Thanks
> Wey
> 
> >  drivers/net/wireless/iwlwifi/iwl-agn-lib.c  |    2 +-
> >  drivers/net/wireless/iwlwifi/iwl3945-base.c |    2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> > index 9dd9e64..8fd00a6 100644
> > --- a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> > +++ b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> > @@ -1411,7 +1411,7 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
> >  	clear_bit(STATUS_SCAN_HW, &priv->status);
> >  	clear_bit(STATUS_SCANNING, &priv->status);
> >  	/* inform mac80211 scan aborted */
> > -	queue_work(priv->workqueue, &priv->scan_completed);
> > +	queue_work(priv->workqueue, &priv->abort_scan);

Unfortunately this patch is not right thing to do. If you look at
abort_scan work, it do nothing if STATUS_SCAN_ABORTING bit is not set.
That's wrong because we have to complete scan (with abort == true).
If STATUS_SCAN_ABORTING will be set, abort_work will send scan cancel
commands to hardware what is wrong if scan was not started yet.

What we can eventually do, except apply iwl-scan rewrite from
wireless-testing, is something like that:
 
iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)

  clear_bit(STATUS_SCAN_HW, &priv->status); 
  clear_bit(STATUS_SCANNING, &priv->status); 
  /* inform mac80211 scan aborted */ 
  set_bit(STATUS_SCAN_ABORTING, &priv->status);
  queue_work(priv->workqueue, &priv->scan_completed);

ieee80211_scan_completed
  
  if (!internal) {
     bool aborted = test_bit(STATUS_SCAN_ABORTING, &priv->status);
     ieee80211_scan_completed(priv->hw, aborted);

  }

However, I do not think we should go with that to -stable (below
2.6.36). IIRC warnings showed up in current 2.6.36-rc, because of
some other changes in the code.

Stanislaw
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html