* potential null deref in iwlagn_request_scan()?
@ 2010-07-21 22:16 Dan Carpenter
2010-07-22 19:28 ` John W. Linville
2010-07-22 19:28 ` [PATCH] iwlwifi: assume vif is NULL for internal scans and non-NULL otherwise John W. Linville
0 siblings, 2 replies; 4+ messages in thread
From: Dan Carpenter @ 2010-07-21 22:16 UTC (permalink / raw)
To: johannes.berg; +Cc: linux-wireless
Smatch complains about iwlagn_request_scan().
drivers/net/wireless/iwlwifi/iwl-agn-lib.c +1354 iwlagn_request_scan(204)
error: we previously assumed 'vif' could be null.
1351 if (!priv->is_internal_short_scan) {
1352 cmd_len = iwl_fill_probe_req(priv,
1353 (struct ieee80211_mgmt *)scan->data,
1354 vif->addr,
^^^^^^^^^
1355 priv->scan_request->ie,
1356 priv->scan_request->ie_len,
1357 IWL_MAX_SCAN_SIZE - sizeof(*scan));
1358 } else {
1359 /* use bcast addr, will not be transmitted but must be valid */
This was added in 3a0b9aad0a8166e9f "iwlwifi: use virtual interface
address for scan". Prior to that commit the function assumed that vif
could be NULL throughout.
I don't know the code well enough to know what to do about this.
Also the same thing for:
drivers/net/wireless/iwlwifi/iwl3945-base.c +2963 iwl3945_request_scan(158)
error: we previously assumed 'vif' could be null.
2962 if (!priv->is_internal_short_scan) {
2963 scan->tx_cmd.len = cpu_to_le16(
2964 iwl_fill_probe_req(priv,
2965 (struct ieee80211_mgmt *)scan->data,
2966 vif->addr,
^^^^^^^^^
2967 priv->scan_request->ie,
2968 priv->scan_request->ie_len,
2969 IWL_MAX_SCAN_SIZE - sizeof(*scan)));
2970 } else {
This is from the above commit as well.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: potential null deref in iwlagn_request_scan()?
2010-07-21 22:16 potential null deref in iwlagn_request_scan()? Dan Carpenter
@ 2010-07-22 19:28 ` John W. Linville
2010-07-22 19:28 ` [PATCH] iwlwifi: assume vif is NULL for internal scans and non-NULL otherwise John W. Linville
1 sibling, 0 replies; 4+ messages in thread
From: John W. Linville @ 2010-07-22 19:28 UTC (permalink / raw)
To: Dan Carpenter; +Cc: johannes.berg, linux-wireless
On Thu, Jul 22, 2010 at 12:16:16AM +0200, Dan Carpenter wrote:
> Smatch complains about iwlagn_request_scan().
>
> drivers/net/wireless/iwlwifi/iwl-agn-lib.c +1354 iwlagn_request_scan(204)
> error: we previously assumed 'vif' could be null.
>
> 1351 if (!priv->is_internal_short_scan) {
> 1352 cmd_len = iwl_fill_probe_req(priv,
> 1353 (struct ieee80211_mgmt *)scan->data,
> 1354 vif->addr,
> ^^^^^^^^^
>
> 1355 priv->scan_request->ie,
> 1356 priv->scan_request->ie_len,
> 1357 IWL_MAX_SCAN_SIZE - sizeof(*scan));
> 1358 } else {
> 1359 /* use bcast addr, will not be transmitted but must be valid */
>
> This was added in 3a0b9aad0a8166e9f "iwlwifi: use virtual interface
> address for scan". Prior to that commit the function assumed that vif
> could be NULL throughout.
>
> I don't know the code well enough to know what to do about this.
>
> Also the same thing for:
> drivers/net/wireless/iwlwifi/iwl3945-base.c +2963 iwl3945_request_scan(158)
> error: we previously assumed 'vif' could be null.
I think both of these are more-or-less OK. It looks like the only
time vif would be NULL is in the "priv->is_internal_short_scan ==
true" case. I'll send a patch, but I don't know if it really matters.
John
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] iwlwifi: assume vif is NULL for internal scans and non-NULL otherwise
2010-07-21 22:16 potential null deref in iwlagn_request_scan()? Dan Carpenter
2010-07-22 19:28 ` John W. Linville
@ 2010-07-22 19:28 ` John W. Linville
2010-07-22 19:36 ` Guy, Wey-Yi
1 sibling, 1 reply; 4+ messages in thread
From: John W. Linville @ 2010-07-22 19:28 UTC (permalink / raw)
To: linux-wireless; +Cc: Dan Carpenter, John W. Linville
The current practice of checking vif for NULL in one place but not
another seems to confuse some static checkers, smatch in particular.
Since vif will only be NULL in the case of internal scans, adjust the
checks accordingly.
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
drivers/net/wireless/iwlwifi/iwl-agn-lib.c | 5 ++++-
drivers/net/wireless/iwlwifi/iwl3945-base.c | 5 ++++-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
index 74623e0..0ca0df4 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
@@ -1234,7 +1234,10 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
IWL_DEBUG_INFO(priv, "Scanning while associated...\n");
spin_lock_irqsave(&priv->lock, flags);
- interval = vif ? vif->bss_conf.beacon_int : 0;
+ if (priv->is_internal_short_scan)
+ interval = 0;
+ else
+ interval = vif->bss_conf.beacon_int;
spin_unlock_irqrestore(&priv->lock, flags);
scan->suspend_time = 0;
diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index 8eb3471..b102bab 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -2883,7 +2883,10 @@ void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
IWL_DEBUG_INFO(priv, "Scanning while associated...\n");
spin_lock_irqsave(&priv->lock, flags);
- interval = vif ? vif->bss_conf.beacon_int : 0;
+ if (priv->is_internal_short_scan)
+ interval = 0;
+ else
+ interval = vif->bss_conf.beacon_int;
spin_unlock_irqrestore(&priv->lock, flags);
scan->suspend_time = 0;
--
1.7.1.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] iwlwifi: assume vif is NULL for internal scans and non-NULL otherwise
2010-07-22 19:28 ` [PATCH] iwlwifi: assume vif is NULL for internal scans and non-NULL otherwise John W. Linville
@ 2010-07-22 19:36 ` Guy, Wey-Yi
0 siblings, 0 replies; 4+ messages in thread
From: Guy, Wey-Yi @ 2010-07-22 19:36 UTC (permalink / raw)
To: John W. Linville; +Cc: linux-wireless, Dan Carpenter
Hi John,
On Thu, 2010-07-22 at 12:28 -0700, John W. Linville wrote:
> The current practice of checking vif for NULL in one place but not
> another seems to confuse some static checkers, smatch in particular.
> Since vif will only be NULL in the case of internal scans, adjust the
> checks accordingly.
>
> Signed-off-by: John W. Linville <linville@tuxdriver.com>
> ---
> drivers/net/wireless/iwlwifi/iwl-agn-lib.c | 5 ++++-
> drivers/net/wireless/iwlwifi/iwl3945-base.c | 5 ++++-
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> index 74623e0..0ca0df4 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> @@ -1234,7 +1234,10 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
>
> IWL_DEBUG_INFO(priv, "Scanning while associated...\n");
> spin_lock_irqsave(&priv->lock, flags);
> - interval = vif ? vif->bss_conf.beacon_int : 0;
> + if (priv->is_internal_short_scan)
> + interval = 0;
> + else
> + interval = vif->bss_conf.beacon_int;
> spin_unlock_irqrestore(&priv->lock, flags);
>
> scan->suspend_time = 0;
> diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> index 8eb3471..b102bab 100644
> --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
> +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> @@ -2883,7 +2883,10 @@ void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
> IWL_DEBUG_INFO(priv, "Scanning while associated...\n");
>
> spin_lock_irqsave(&priv->lock, flags);
> - interval = vif ? vif->bss_conf.beacon_int : 0;
> + if (priv->is_internal_short_scan)
> + interval = 0;
> + else
> + interval = vif->bss_conf.beacon_int;
> spin_unlock_irqrestore(&priv->lock, flags);
>
> scan->suspend_time = 0;
Make sense, Thanks.
Wey
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-07-22 19:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-21 22:16 potential null deref in iwlagn_request_scan()? Dan Carpenter
2010-07-22 19:28 ` John W. Linville
2010-07-22 19:28 ` [PATCH] iwlwifi: assume vif is NULL for internal scans and non-NULL otherwise John W. Linville
2010-07-22 19:36 ` Guy, Wey-Yi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).