iwlwifi injection bug

iwlwifi injection bug

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 628 bytes --]

I don't know why you want to disallow sending frames when in monitor
mode (especially since reportedly it works), but this is not a good way
to go about it:

static int iwl4965_mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
{
        struct iwl_priv *priv = hw->priv;

        IWL_DEBUG_MAC80211("enter\n");

        if (priv->iw_mode == IEEE80211_IF_TYPE_MNTR) {
                IWL_DEBUG_MAC80211("leave - monitor\n");
                return -1;
        }

You see, this will cause mac80211 to resubmit the frame and most likely
will totally mess up queue handling and various other things.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: iwlwifi injection bug

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1250 bytes --]

On Fri, 2008-05-16 at 17:12 +0200, Stefanik Gábor wrote:
> On Fri, May 16, 2008 at 3:47 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > I don't know why you want to disallow sending frames when in monitor
> > mode (especially since reportedly it works), but this is not a good way
> > to go about it:
> >
> > static int iwl4965_mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
> > {
> >        struct iwl_priv *priv = hw->priv;
> >
> >        IWL_DEBUG_MAC80211("enter\n");
> >
> >        if (priv->iw_mode == IEEE80211_IF_TYPE_MNTR) {
> >                IWL_DEBUG_MAC80211("leave - monitor\n");
> >                return -1;
> >        }
> >
> > You see, this will cause mac80211 to resubmit the frame and most likely
> > will totally mess up queue handling and various other things.
> >
> > johannes
> >
> 
> Hmm... thanks for the hint! I was chasing a weird Radiotap bug that
> shown up when I tried to use packetspammer or aireplay on a monitor
> interface with the latest iwlwifi git. The symptoms ranged from just
> not transmitting to hangs and kernel panics. It appears to mess up
> more than just TX/RX queues.

Doesn't surprise me, but I'd be interested in the panics nonetheless.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: iwlwifi injection bug

[Zhu Yi]

On Fri, 2008-05-16 at 15:47 +0200, Johannes Berg wrote:
> I don't know why you want to disallow sending frames when in monitor
> mode (especially since reportedly it works), but this is not a good
> way
> to go about it:
> 
> static int iwl4965_mac_tx(struct ieee80211_hw *hw, struct sk_buff
> *skb)
> {
>         struct iwl_priv *priv = hw->priv;
> 
>         IWL_DEBUG_MAC80211("enter\n");
> 
>         if (priv->iw_mode == IEEE80211_IF_TYPE_MNTR) {
>                 IWL_DEBUG_MAC80211("leave - monitor\n");
>                 return -1;
>         }
> 
> You see, this will cause mac80211 to resubmit the frame and most
> likely will totally mess up queue handling and various other things.

The monitor mode the driver is using currently is purely a "sniffer". We
will look into other promiscuous mode supported by the firmware to see
if we can enable that.

Thanks,
-yi

Re: iwlwifi injection bug

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 639 bytes --]


> >         if (priv->iw_mode == IEEE80211_IF_TYPE_MNTR) {
> >                 IWL_DEBUG_MAC80211("leave - monitor\n");
> >                 return -1;
> >         }
> > 
> > You see, this will cause mac80211 to resubmit the frame and most
> > likely will totally mess up queue handling and various other things.
> 
> The monitor mode the driver is using currently is purely a "sniffer". We
> will look into other promiscuous mode supported by the firmware to see
> if we can enable that.

Until then, however, you should return 0 and free the skb there to avoid
messing up the whole networking layer's queuing.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: iwlwifi injection bug

[Zhu Yi]

On Tue, 2008-05-20 at 11:13 +0200, Johannes Berg wrote:
> 
> Until then, however, you should return 0 and free the skb there to
> avoid messing up the whole networking layer's queuing.

I see the real problem now. Will do. Thanks!

-yi

Re: iwlwifi injection bug

[Stefanik Gábor]

On Mon, May 19, 2008 at 10:15 PM, Zhu Yi <yi.zhu@intel.com> wrote:
> On Fri, 2008-05-16 at 15:47 +0200, Johannes Berg wrote:
>> I don't know why you want to disallow sending frames when in monitor
>> mode (especially since reportedly it works), but this is not a good
>> way
>> to go about it:
>>
>> static int iwl4965_mac_tx(struct ieee80211_hw *hw, struct sk_buff
>> *skb)
>> {
>>         struct iwl_priv *priv = hw->priv;
>>
>>         IWL_DEBUG_MAC80211("enter\n");
>>
>>         if (priv->iw_mode == IEEE80211_IF_TYPE_MNTR) {
>>                 IWL_DEBUG_MAC80211("leave - monitor\n");
>>                 return -1;
>>         }
>>
>> You see, this will cause mac80211 to resubmit the frame and most
>> likely will totally mess up queue handling and various other things.
>
> The monitor mode the driver is using currently is purely a "sniffer". We
> will look into other promiscuous mode supported by the firmware to see
> if we can enable that.
>
> Thanks,
> -yi
>

That mode is not really a sniffer, since I can perfectly use packet
injection using this patch; packetspammer, aireplay-ng and mdk3 all
work, at least for 3945 (4965 not tested because I don't own one):

diff -rp -U 8 compat-wireless-2008-05-20/drivers/net/wireless/iwlwifi/iwl3945-base.c
compat-wireless-2008-05-20.bak/drivers/net/wireless/iwlwifi/iwl3945-base.c
--- compat-wireless-2008-05-20/drivers/net/wireless/iwlwifi/iwl3945-base.c	2008-05-20
05:05:29.000000000 -0400
+++ compat-wireless-2008-05-20.bak/drivers/net/wireless/iwlwifi/iwl3945-base.c	2008-05-20
09:28:05.321889249 -0400
@@ -2579,21 +2579,16 @@ static int iwl3945_tx_skb(struct iwl3945
 	int rc;

 	spin_lock_irqsave(&priv->lock, flags);
 	if (iwl3945_is_rfkill(priv)) {
 		IWL_DEBUG_DROP("Dropping - RF KILL\n");
 		goto drop_unlock;
 	}

-	if (!priv->vif) {
-		IWL_DEBUG_DROP("Dropping - !priv->vif\n");
-		goto drop_unlock;
-	}
-
 	if ((ctl->tx_rate->hw_value & 0xFF) == IWL_INVALID_RATE) {
 		IWL_ERROR("ERROR: No TX rate available.\n");
 		goto drop_unlock;
 	}

 	unicast = !is_multicast_ether_addr(hdr->addr1);
 	id = 0;

@@ -2606,33 +2601,36 @@ static int iwl3945_tx_skb(struct iwl3945
 		IWL_DEBUG_TX("Sending ASSOC frame\n");
 	else if (ieee80211_is_reassoc_request(fc))
 		IWL_DEBUG_TX("Sending REASSOC frame\n");
 #endif

 	/* drop all data frame if we are not associated */
 	if ((!iwl3945_is_associated(priv) ||
 	     ((priv->iw_mode == IEEE80211_IF_TYPE_STA) && !priv->assoc_id)) &&
-	    ((fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_DATA)) {
+	    ((fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_DATA) && priv->vif) {
 		IWL_DEBUG_DROP("Dropping - !iwl3945_is_associated\n");
 		goto drop_unlock;
 	}

 	spin_unlock_irqrestore(&priv->lock, flags);

 	hdr_len = ieee80211_get_hdrlen(fc);

 	/* Find (or create) index into station table for destination station */
 	sta_id = iwl3945_get_sta_id(priv, hdr);
 	if (sta_id == IWL_INVALID_STATION) {
 		DECLARE_MAC_BUF(mac);

-		IWL_DEBUG_DROP("Dropping - INVALID STATION: %s\n",
-			       print_mac(mac, hdr->addr1));
-		goto drop;
+		if (priv->vif) {
+			IWL_DEBUG_DROP("Dropping - INVALID STATION: %s\n",
+				       print_mac(mac, hdr->addr1));
+			goto drop;
+		}
+		sta_id = priv->hw_setting.bcast_sta_id;
 	}

 	IWL_DEBUG_RATE("station Id %d\n", sta_id);

 	qc = ieee80211_get_qos_ctrl(hdr);
 	if (qc) {
 		u8 tid = (u8)(le16_to_cpu(*qc) & 0xf);
 		seq_number = priv->stations[sta_id].tid[tid].seq_number &
@@ -6692,17 +6690,17 @@ static void iwl3945_mac_stop(struct ieee

 static int iwl3945_mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb,
 		      struct ieee80211_tx_control *ctl)
 {
 	struct iwl3945_priv *priv = hw->priv;

 	IWL_DEBUG_MAC80211("enter\n");

-	if (priv->iw_mode == IEEE80211_IF_TYPE_MNTR) {
+	if (priv->iw_mode == IEEE80211_IF_TYPE_MNTR && 0) {
 		IWL_DEBUG_MAC80211("leave - monitor\n");
 		return -1;
 	}

 	IWL_DEBUG_TX("dev->xmit(%d bytes) at rate 0x%02x\n", skb->len,
 		     ctl->tx_rate->bitrate);

 	if (iwl3945_tx_skb(priv, skb, ctl))
diff -rp -U 8 compat-wireless-2008-05-20/drivers/net/wireless/iwlwifi/iwl4965-base.c
compat-wireless-2008-05-20.bak/drivers/net/wireless/iwlwifi/iwl4965-base.c
--- compat-wireless-2008-05-20/drivers/net/wireless/iwlwifi/iwl4965-base.c	2008-05-20
05:05:29.000000000 -0400
+++ compat-wireless-2008-05-20.bak/drivers/net/wireless/iwlwifi/iwl4965-base.c	2008-05-20
09:28:05.325889557 -0400
@@ -1784,21 +1784,16 @@ static int iwl4965_tx_skb(struct iwl_pri
 	int rc;

 	spin_lock_irqsave(&priv->lock, flags);
 	if (iwl_is_rfkill(priv)) {
 		IWL_DEBUG_DROP("Dropping - RF KILL\n");
 		goto drop_unlock;
 	}

-	if (!priv->vif) {
-		IWL_DEBUG_DROP("Dropping - !priv->vif\n");
-		goto drop_unlock;
-	}
-
 	if ((ctl->tx_rate->hw_value & 0xFF) == IWL_INVALID_RATE) {
 		IWL_ERROR("ERROR: No TX rate available.\n");
 		goto drop_unlock;
 	}

 	unicast = !is_multicast_ether_addr(hdr->addr1);
 	id = 0;

@@ -1810,35 +1805,37 @@ static int iwl4965_tx_skb(struct iwl_pri
 	else if (ieee80211_is_assoc_request(fc))
 		IWL_DEBUG_TX("Sending ASSOC frame\n");
 	else if (ieee80211_is_reassoc_request(fc))
 		IWL_DEBUG_TX("Sending REASSOC frame\n");
 #endif

 	/* drop all data frame if we are not associated */
 	if (((fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_DATA) &&
-	   (!iwl_is_associated(priv) ||
+	   priv->vif && (!iwl_is_associated(priv) ||
 	    ((priv->iw_mode == IEEE80211_IF_TYPE_STA) && !priv->assoc_id) ||
 	    !priv->assoc_station_added)) {
 		IWL_DEBUG_DROP("Dropping - !iwl_is_associated\n");
 		goto drop_unlock;
 	}

 	spin_unlock_irqrestore(&priv->lock, flags);

 	hdr_len = ieee80211_get_hdrlen(fc);

 	/* Find (or create) index into station table for destination station */
 	sta_id = iwl4965_get_sta_id(priv, hdr);
 	if (sta_id == IWL_INVALID_STATION) {
 		DECLARE_MAC_BUF(mac);

-		IWL_DEBUG_DROP("Dropping - INVALID STATION: %s\n",
-			       print_mac(mac, hdr->addr1));
-		goto drop;
+		if (priv->vif) {
+			IWL_DEBUG_DROP("Dropping - INVALID STATION: %s\n", print_mac(mac,
hdr->addr1));
+			goto drop;
+		}
+		sta_id = priv->hw_params.bcast_sta_id;
 	}

 	IWL_DEBUG_TX("station Id %d\n", sta_id);

 	qc = ieee80211_get_qos_ctrl(hdr);
 	if (qc) {
 		u8 tid = (u8)(le16_to_cpu(*qc) & 0xf);
 		seq_number = priv->stations[sta_id].tid[tid].seq_number &
@@ -5110,17 +5107,17 @@ static void iwl4965_mac_stop(struct ieee

 static int iwl4965_mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb,
 		      struct ieee80211_tx_control *ctl)
 {
 	struct iwl_priv *priv = hw->priv;

 	IWL_DEBUG_MAC80211("enter\n");

-	if (priv->iw_mode == IEEE80211_IF_TYPE_MNTR) {
+	if (priv->iw_mode == IEEE80211_IF_TYPE_MNTR && 0) {
 		IWL_DEBUG_MAC80211("leave - monitor\n");
 		return -1;
 	}

 	IWL_DEBUG_TX("dev->xmit(%d bytes) at rate 0x%02x\n", skb->len,
 		     ctl->tx_rate->bitrate);

 	if (iwl4965_tx_skb(priv, skb, ctl))

Re: iwlwifi injection bug

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 2097 bytes --]


>  	/* drop all data frame if we are not associated */
>  	if ((!iwl3945_is_associated(priv) ||
>  	     ((priv->iw_mode == IEEE80211_IF_TYPE_STA) && !priv->assoc_id)) &&
> -	    ((fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_DATA)) {
> +	    ((fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_DATA) && priv->vif) {
>  		IWL_DEBUG_DROP("Dropping - !iwl3945_is_associated\n");
>  		goto drop_unlock;
>  	}

That code is completely bogus anyway IMO, it should just be removed.

>  	/* Find (or create) index into station table for destination station */
>  	sta_id = iwl3945_get_sta_id(priv, hdr);
>  	if (sta_id == IWL_INVALID_STATION) {
>  		DECLARE_MAC_BUF(mac);
> 
> -		IWL_DEBUG_DROP("Dropping - INVALID STATION: %s\n",
> -			       print_mac(mac, hdr->addr1));
> -		goto drop;
> +		if (priv->vif) {
> +			IWL_DEBUG_DROP("Dropping - INVALID STATION: %s\n",
> +				       print_mac(mac, hdr->addr1));
> +			goto drop;
> +		}
> +		sta_id = priv->hw_setting.bcast_sta_id;
>  	}

Same here, why drop a frame if there's no station for it, that just
means you can't do hw encryption which, well, cannot happen anyway since
mac80211 would tell you about the station for hw crypto.

> -	if (priv->iw_mode == IEEE80211_IF_TYPE_MNTR) {
> +	if (priv->iw_mode == IEEE80211_IF_TYPE_MNTR && 0) {
>  		IWL_DEBUG_MAC80211("leave - monitor\n");
>  		return -1;
>  	}

That's the code I was complaining about. Just kill it if it works then.

>  	/* drop all data frame if we are not associated */
>  	if (((fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_DATA) &&
> -	   (!iwl_is_associated(priv) ||
> +	   priv->vif && (!iwl_is_associated(priv) ||
>  	    ((priv->iw_mode == IEEE80211_IF_TYPE_STA) && !priv->assoc_id) ||
>  	    !priv->assoc_station_added)) {
>  		IWL_DEBUG_DROP("Dropping - !iwl_is_associated\n");
>  		goto drop_unlock;
>  	}

Same code, other driver. What are you doing poking with the 802.11
frames you're supposed to transmit anyway?

I wish you guys at Intel would trust mac80211 and not try to work around
it all the time.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: iwlwifi injection bug

[Stefanik Gábor]

(Resend of previous mail, his time also sending to the list.)
On Tue, May 20, 2008 at 8:38 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
>
>> <snip>
>
> That code is completely bogus anyway IMO, it should just be removed.
>
>> <snip>
>
> Same here, why drop a frame if there's no station for it, that just
> means you can't do hw encryption which, well, cannot happen anyway since
> mac80211 would tell you about the station for hw crypto.
>
>> <snip>
>
> That's the code I was complaining about. Just kill it if it works then.
>
>> <snip>
>
> Same code, other driver. What are you doing poking with the 802.11
> frames you're supposed to transmit anyway?
>
> I wish you guys at Intel would trust mac80211 and not try to work around
> it all the time.
>
> johannes
>

Patch with nits addressed, a syslog issue fixed and also with a
bracketing fix in 3945:

diff -rp -U 8 compat-wireless-2008-05-20.orig/drivers/net/wireless/iwlwifi/iwl3945-base.c
compat-wireless-2008-05-20/drivers/net/wireless/iwlwifi/iwl3945-base.c
--- compat-wireless-2008-05-20.orig/drivers/net/wireless/iwlwifi/iwl3945-base.c	2008-05-20
05:05:29.000000000 -0400
+++ compat-wireless-2008-05-20/drivers/net/wireless/iwlwifi/iwl3945-base.c	2008-05-20
08:54:52.819664502 -0400
@@ -2523,17 +2523,17 @@ static int iwl3945_get_sta_id(struct iwl
 	case IEEE80211_IF_TYPE_AP:
 		sta_id = iwl3945_hw_find_station(priv, hdr->addr1);
 		if (sta_id != IWL_INVALID_STATION)
 			return sta_id;
 		return priv->hw_setting.bcast_sta_id;

 	/* If this frame is going out to an IBSS network, find the station,
 	 * or create a new station table entry */
-	case IEEE80211_IF_TYPE_IBSS: {
+	case IEEE80211_IF_TYPE_IBSS:
 		DECLARE_MAC_BUF(mac);

 		/* Create new station table entry */
 		sta_id = iwl3945_hw_find_station(priv, hdr->addr1);
 		if (sta_id != IWL_INVALID_STATION)
 			return sta_id;

 		sta_id = iwl3945_add_station(priv, hdr->addr1, 0, CMD_ASYNC);
@@ -2541,17 +2541,20 @@ static int iwl3945_get_sta_id(struct iwl
 		if (sta_id != IWL_INVALID_STATION)
 			return sta_id;

 		IWL_DEBUG_DROP("Station %s not in station map. "
 			       "Defaulting to broadcast...\n",
 			       print_mac(mac, hdr->addr1));
 		iwl3945_print_hex_dump(IWL_DL_DROP, (u8 *) hdr, sizeof(*hdr));
 		return priv->hw_setting.bcast_sta_id;
-	}
+
+	/* If we are in monitor mode, use BCAST */
+	case IEEE80211_IF_TYPE_MNTR:
+		return priv->hw_params.bcast_sta_id;
 	default:
 		IWL_WARNING("Unknown mode of operation: %d", priv->iw_mode);
 		return priv->hw_setting.bcast_sta_id;
 	}
 }

 /*
  * start REPLY_TX command process
@@ -2579,21 +2582,16 @@ static int iwl3945_tx_skb(struct iwl3945
 	int rc;

 	spin_lock_irqsave(&priv->lock, flags);
 	if (iwl3945_is_rfkill(priv)) {
 		IWL_DEBUG_DROP("Dropping - RF KILL\n");
 		goto drop_unlock;
 	}

-	if (!priv->vif) {
-		IWL_DEBUG_DROP("Dropping - !priv->vif\n");
-		goto drop_unlock;
-	}
-
 	if ((ctl->tx_rate->hw_value & 0xFF) == IWL_INVALID_RATE) {
 		IWL_ERROR("ERROR: No TX rate available.\n");
 		goto drop_unlock;
 	}

 	unicast = !is_multicast_ether_addr(hdr->addr1);
 	id = 0;

@@ -2603,37 +2601,22 @@ static int iwl3945_tx_skb(struct iwl3945
 	if (ieee80211_is_auth(fc))
 		IWL_DEBUG_TX("Sending AUTH frame\n");
 	else if (ieee80211_is_assoc_request(fc))
 		IWL_DEBUG_TX("Sending ASSOC frame\n");
 	else if (ieee80211_is_reassoc_request(fc))
 		IWL_DEBUG_TX("Sending REASSOC frame\n");
 #endif

-	/* drop all data frame if we are not associated */
-	if ((!iwl3945_is_associated(priv) ||
-	     ((priv->iw_mode == IEEE80211_IF_TYPE_STA) && !priv->assoc_id)) &&
-	    ((fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_DATA)) {
-		IWL_DEBUG_DROP("Dropping - !iwl3945_is_associated\n");
-		goto drop_unlock;
-	}
-
 	spin_unlock_irqrestore(&priv->lock, flags);

 	hdr_len = ieee80211_get_hdrlen(fc);

 	/* Find (or create) index into station table for destination station */
 	sta_id = iwl3945_get_sta_id(priv, hdr);
-	if (sta_id == IWL_INVALID_STATION) {
-		DECLARE_MAC_BUF(mac);
-
-		IWL_DEBUG_DROP("Dropping - INVALID STATION: %s\n",
-			       print_mac(mac, hdr->addr1));
-		goto drop;
-	}

 	IWL_DEBUG_RATE("station Id %d\n", sta_id);

 	qc = ieee80211_get_qos_ctrl(hdr);
 	if (qc) {
 		u8 tid = (u8)(le16_to_cpu(*qc) & 0xf);
 		seq_number = priv->stations[sta_id].tid[tid].seq_number &
 				IEEE80211_SCTL_SEQ;
@@ -6692,21 +6675,16 @@ static void iwl3945_mac_stop(struct ieee

 static int iwl3945_mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb,
 		      struct ieee80211_tx_control *ctl)
 {
 	struct iwl3945_priv *priv = hw->priv;

 	IWL_DEBUG_MAC80211("enter\n");

-	if (priv->iw_mode == IEEE80211_IF_TYPE_MNTR) {
-		IWL_DEBUG_MAC80211("leave - monitor\n");
-		return -1;
-	}
-
 	IWL_DEBUG_TX("dev->xmit(%d bytes) at rate 0x%02x\n", skb->len,
 		     ctl->tx_rate->bitrate);

 	if (iwl3945_tx_skb(priv, skb, ctl))
 		dev_kfree_skb_any(skb);

 	IWL_DEBUG_MAC80211("leave\n");
 	return 0;
diff -rp -U 8 compat-wireless-2008-05-20.orig/drivers/net/wireless/iwlwifi/iwl4965-base.c
compat-wireless-2008-05-20/drivers/net/wireless/iwlwifi/iwl4965-base.c
--- compat-wireless-2008-05-20.orig/drivers/net/wireless/iwlwifi/iwl4965-base.c	2008-05-20
05:05:29.000000000 -0400
+++ compat-wireless-2008-05-20/drivers/net/wireless/iwlwifi/iwl4965-base.c	2008-05-20
08:55:20.689530273 -0400
@@ -1746,16 +1746,19 @@ static int iwl4965_get_sta_id(struct iwl
 			return sta_id;

 		IWL_DEBUG_DROP("Station %s not in station map. "
 			       "Defaulting to broadcast...\n",
 			       print_mac(mac, hdr->addr1));
 		iwl_print_hex_dump(priv, IWL_DL_DROP, (u8 *) hdr, sizeof(*hdr));
 		return priv->hw_params.bcast_sta_id;

+	/* If we are in monitor mode, use BCAST */
+	case IEEE80211_IF_TYPE_MNTR:
+		return priv->hw_params.bcast_sta_id;
 	default:
 		IWL_WARNING("Unknown mode of operation: %d", priv->iw_mode);
 		return priv->hw_params.bcast_sta_id;
 	}
 }

 /*
  * start REPLY_TX command process
@@ -1784,21 +1787,16 @@ static int iwl4965_tx_skb(struct iwl_pri
 	int rc;

 	spin_lock_irqsave(&priv->lock, flags);
 	if (iwl_is_rfkill(priv)) {
 		IWL_DEBUG_DROP("Dropping - RF KILL\n");
 		goto drop_unlock;
 	}

-	if (!priv->vif) {
-		IWL_DEBUG_DROP("Dropping - !priv->vif\n");
-		goto drop_unlock;
-	}
-
 	if ((ctl->tx_rate->hw_value & 0xFF) == IWL_INVALID_RATE) {
 		IWL_ERROR("ERROR: No TX rate available.\n");
 		goto drop_unlock;
 	}

 	unicast = !is_multicast_ether_addr(hdr->addr1);
 	id = 0;

@@ -1808,38 +1806,22 @@ static int iwl4965_tx_skb(struct iwl_pri
 	if (ieee80211_is_auth(fc))
 		IWL_DEBUG_TX("Sending AUTH frame\n");
 	else if (ieee80211_is_assoc_request(fc))
 		IWL_DEBUG_TX("Sending ASSOC frame\n");
 	else if (ieee80211_is_reassoc_request(fc))
 		IWL_DEBUG_TX("Sending REASSOC frame\n");
 #endif

-	/* drop all data frame if we are not associated */
-	if (((fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_DATA) &&
-	   (!iwl_is_associated(priv) ||
-	    ((priv->iw_mode == IEEE80211_IF_TYPE_STA) && !priv->assoc_id) ||
-	    !priv->assoc_station_added)) {
-		IWL_DEBUG_DROP("Dropping - !iwl_is_associated\n");
-		goto drop_unlock;
-	}
-
 	spin_unlock_irqrestore(&priv->lock, flags);

 	hdr_len = ieee80211_get_hdrlen(fc);

 	/* Find (or create) index into station table for destination station */
 	sta_id = iwl4965_get_sta_id(priv, hdr);
-	if (sta_id == IWL_INVALID_STATION) {
-		DECLARE_MAC_BUF(mac);
-
-		IWL_DEBUG_DROP("Dropping - INVALID STATION: %s\n",
-			       print_mac(mac, hdr->addr1));
-		goto drop;
-	}

 	IWL_DEBUG_TX("station Id %d\n", sta_id);

 	qc = ieee80211_get_qos_ctrl(hdr);
 	if (qc) {
 		u8 tid = (u8)(le16_to_cpu(*qc) & 0xf);
 		seq_number = priv->stations[sta_id].tid[tid].seq_number &
 				IEEE80211_SCTL_SEQ;
@@ -5110,21 +5092,16 @@ static void iwl4965_mac_stop(struct ieee

 static int iwl4965_mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb,
 		      struct ieee80211_tx_control *ctl)
 {
 	struct iwl_priv *priv = hw->priv;

 	IWL_DEBUG_MAC80211("enter\n");

-	if (priv->iw_mode == IEEE80211_IF_TYPE_MNTR) {
-		IWL_DEBUG_MAC80211("leave - monitor\n");
-		return -1;
-	}
-
 	IWL_DEBUG_TX("dev->xmit(%d bytes) at rate 0x%02x\n", skb->len,
 		     ctl->tx_rate->bitrate);

 	if (iwl4965_tx_skb(priv, skb, ctl))
 		dev_kfree_skb_any(skb);

 	IWL_DEBUG_MAC80211("leave\n");
 	return 0;

Re: iwlwifi injection bug

[Tomas Winkler]

On Tue, May 20, 2008 at 4:14 PM, Stefanik G=E1bor <netrolller.3d@gmail.=
com> wrote:
> (Resend of previous mail, his time also sending to the list.)
> On Tue, May 20, 2008 at 8:38 AM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
>>
>>> <snip>
>>
>> That code is completely bogus anyway IMO, it should just be removed.
>>
>>> <snip>
>>
>> Same here, why drop a frame if there's no station for it, that just
>> means you can't do hw encryption which, well, cannot happen anyway s=
ince
>> mac80211 would tell you about the station for hw crypto.
>>
>>> <snip>
>>
>> That's the code I was complaining about. Just kill it if it works th=
en.
>>
>>> <snip>
>>
>> Same code, other driver. What are you doing poking with the 802.11
>> frames you're supposed to transmit anyway?
>>
>> I wish you guys at Intel would trust mac80211 and not try to work ar=
ound
>> it all the time.
>>
>> johannes
>>
>
> Patch with nits addressed, a syslog issue fixed and also with a
> bracketing fix in 3945:
>
> diff -rp -U 8 compat-wireless-2008-05-20.orig/drivers/net/wireless/iw=
lwifi/iwl3945-base.c
> compat-wireless-2008-05-20/drivers/net/wireless/iwlwifi/iwl3945-base.=
c
> --- compat-wireless-2008-05-20.orig/drivers/net/wireless/iwlwifi/iwl3=
945-base.c 2008-05-20
> 05:05:29.000000000 -0400
> +++ compat-wireless-2008-05-20/drivers/net/wireless/iwlwifi/iwl3945-b=
ase.c      2008-05-20
> 08:54:52.819664502 -0400
> @@ -2523,17 +2523,17 @@ static int iwl3945_get_sta_id(struct iwl
>        case IEEE80211_IF_TYPE_AP:
>                sta_id =3D iwl3945_hw_find_station(priv, hdr->addr1);
>                if (sta_id !=3D IWL_INVALID_STATION)
>                        return sta_id;
>                return priv->hw_setting.bcast_sta_id;
>
>        /* If this frame is going out to an IBSS network, find the sta=
tion,
>         * or create a new station table entry */
> -       case IEEE80211_IF_TYPE_IBSS: {
> +       case IEEE80211_IF_TYPE_IBSS:
>                DECLARE_MAC_BUF(mac);
>
>                /* Create new station table entry */
>                sta_id =3D iwl3945_hw_find_station(priv, hdr->addr1);
>                if (sta_id !=3D IWL_INVALID_STATION)
>                        return sta_id;
>
>                sta_id =3D iwl3945_add_station(priv, hdr->addr1, 0, CM=
D_ASYNC);
> @@ -2541,17 +2541,20 @@ static int iwl3945_get_sta_id(struct iwl
>                if (sta_id !=3D IWL_INVALID_STATION)
>                        return sta_id;
>
>                IWL_DEBUG_DROP("Station %s not in station map. "
>                               "Defaulting to broadcast...\n",
>                               print_mac(mac, hdr->addr1));
>                iwl3945_print_hex_dump(IWL_DL_DROP, (u8 *) hdr, sizeof=
(*hdr));
>                return priv->hw_setting.bcast_sta_id;
> -       }
> +
> +       /* If we are in monitor mode, use BCAST */
> +       case IEEE80211_IF_TYPE_MNTR:
> +               return priv->hw_params.bcast_sta_id;
>        default:
>                IWL_WARNING("Unknown mode of operation: %d", priv->iw_=
mode);
>                return priv->hw_setting.bcast_sta_id;
>        }
>  }
>
This seams Okay.
>  /*
>  * start REPLY_TX command process
> @@ -2579,21 +2582,16 @@ static int iwl3945_tx_skb(struct iwl3945
>        int rc;
>
>        spin_lock_irqsave(&priv->lock, flags);
>        if (iwl3945_is_rfkill(priv)) {
>                IWL_DEBUG_DROP("Dropping - RF KILL\n");
>                goto drop_unlock;
>        }
>
> -       if (!priv->vif) {
> -               IWL_DEBUG_DROP("Dropping - !priv->vif\n");
> -               goto drop_unlock;
> -       }
> -

Not sure about this

>        if ((ctl->tx_rate->hw_value & 0xFF) =3D=3D IWL_INVALID_RATE) {
>                IWL_ERROR("ERROR: No TX rate available.\n");
>                goto drop_unlock;
>        }
>
>        unicast =3D !is_multicast_ether_addr(hdr->addr1);
>        id =3D 0;
>
> @@ -2603,37 +2601,22 @@ static int iwl3945_tx_skb(struct iwl3945
>        if (ieee80211_is_auth(fc))
>                IWL_DEBUG_TX("Sending AUTH frame\n");
>        else if (ieee80211_is_assoc_request(fc))
>                IWL_DEBUG_TX("Sending ASSOC frame\n");
>        else if (ieee80211_is_reassoc_request(fc))
>                IWL_DEBUG_TX("Sending REASSOC frame\n");
>  #endif
>
> -       /* drop all data frame if we are not associated */
> -       if ((!iwl3945_is_associated(priv) ||
> -            ((priv->iw_mode =3D=3D IEEE80211_IF_TYPE_STA) && !priv->=
assoc_id)) &&
> -           ((fc & IEEE80211_FCTL_FTYPE) =3D=3D IEEE80211_FTYPE_DATA)=
) {
> -               IWL_DEBUG_DROP("Dropping - !iwl3945_is_associated\n")=
;
> -               goto drop_unlock;
> -       }
> -
The thing is that mac80211 and uCode are not exactly in sync on
association time.
I'm not sure but this would be also spec violation.

>        spin_unlock_irqrestore(&priv->lock, flags);
>
>        hdr_len =3D ieee80211_get_hdrlen(fc);
>
>        /* Find (or create) index into station table for destination s=
tation */
>        sta_id =3D iwl3945_get_sta_id(priv, hdr);
> -       if (sta_id =3D=3D IWL_INVALID_STATION) {
> -               DECLARE_MAC_BUF(mac);
> -
> -               IWL_DEBUG_DROP("Dropping - INVALID STATION: %s\n",
> -                              print_mac(mac, hdr->addr1));
> -               goto drop;
> -       }
>
send packet with invalid station will certainly crash uCode.

>        IWL_DEBUG_RATE("station Id %d\n", sta_id);
>
>        qc =3D ieee80211_get_qos_ctrl(hdr);
>        if (qc) {
>                u8 tid =3D (u8)(le16_to_cpu(*qc) & 0xf);
>                seq_number =3D priv->stations[sta_id].tid[tid].seq_num=
ber &
>                                IEEE80211_SCTL_SEQ;
> @@ -6692,21 +6675,16 @@ static void iwl3945_mac_stop(struct ieee
>
>  static int iwl3945_mac_tx(struct ieee80211_hw *hw, struct sk_buff *s=
kb,
>                      struct ieee80211_tx_control *ctl)
>  {
>        struct iwl3945_priv *priv =3D hw->priv;
>
>        IWL_DEBUG_MAC80211("enter\n");
>
> -       if (priv->iw_mode =3D=3D IEEE80211_IF_TYPE_MNTR) {
> -               IWL_DEBUG_MAC80211("leave - monitor\n");
> -               return -1;
> -       }
> -
>        IWL_DEBUG_TX("dev->xmit(%d bytes) at rate 0x%02x\n", skb->len,
>                     ctl->tx_rate->bitrate);
>
>        if (iwl3945_tx_skb(priv, skb, ctl))
>                dev_kfree_skb_any(skb);
>
>        IWL_DEBUG_MAC80211("leave\n");
>        return 0;
> diff -rp -U 8 compat-wireless-2008-05-20.orig/drivers/net/wireless/iw=
lwifi/iwl4965-base.c
> compat-wireless-2008-05-20/drivers/net/wireless/iwlwifi/iwl4965-base.=
c
> --- compat-wireless-2008-05-20.orig/drivers/net/wireless/iwlwifi/iwl4=
965-base.c 2008-05-20
> 05:05:29.000000000 -0400
> +++ compat-wireless-2008-05-20/drivers/net/wireless/iwlwifi/iwl4965-b=
ase.c      2008-05-20
> 08:55:20.689530273 -0400
> @@ -1746,16 +1746,19 @@ static int iwl4965_get_sta_id(struct iwl
>                        return sta_id;
>
>                IWL_DEBUG_DROP("Station %s not in station map. "
>                               "Defaulting to broadcast...\n",
>                               print_mac(mac, hdr->addr1));
>                iwl_print_hex_dump(priv, IWL_DL_DROP, (u8 *) hdr, size=
of(*hdr));
>                return priv->hw_params.bcast_sta_id;
>
> +       /* If we are in monitor mode, use BCAST */
> +       case IEEE80211_IF_TYPE_MNTR:
> +               return priv->hw_params.bcast_sta_id;
>        default:
>                IWL_WARNING("Unknown mode of operation: %d", priv->iw_=
mode);
>                return priv->hw_params.bcast_sta_id;
>        }
>  }
>
>  /*
>  * start REPLY_TX command process
> @@ -1784,21 +1787,16 @@ static int iwl4965_tx_skb(struct iwl_pri
>        int rc;
>
>        spin_lock_irqsave(&priv->lock, flags);
>        if (iwl_is_rfkill(priv)) {
>                IWL_DEBUG_DROP("Dropping - RF KILL\n");
>                goto drop_unlock;
>        }
>
> -       if (!priv->vif) {
> -               IWL_DEBUG_DROP("Dropping - !priv->vif\n");
> -               goto drop_unlock;
> -       }
> -
>        if ((ctl->tx_rate->hw_value & 0xFF) =3D=3D IWL_INVALID_RATE) {
>                IWL_ERROR("ERROR: No TX rate available.\n");
>                goto drop_unlock;
>        }
>
>        unicast =3D !is_multicast_ether_addr(hdr->addr1);
>        id =3D 0;
>
> @@ -1808,38 +1806,22 @@ static int iwl4965_tx_skb(struct iwl_pri
>        if (ieee80211_is_auth(fc))
>                IWL_DEBUG_TX("Sending AUTH frame\n");
>        else if (ieee80211_is_assoc_request(fc))
>                IWL_DEBUG_TX("Sending ASSOC frame\n");
>        else if (ieee80211_is_reassoc_request(fc))
>                IWL_DEBUG_TX("Sending REASSOC frame\n");
>  #endif
>
> -       /* drop all data frame if we are not associated */
> -       if (((fc & IEEE80211_FCTL_FTYPE) =3D=3D IEEE80211_FTYPE_DATA)=
 &&
> -          (!iwl_is_associated(priv) ||
> -           ((priv->iw_mode =3D=3D IEEE80211_IF_TYPE_STA) && !priv->a=
ssoc_id) ||
> -           !priv->assoc_station_added)) {
> -               IWL_DEBUG_DROP("Dropping - !iwl_is_associated\n");
> -               goto drop_unlock;
> -       }
> -
>        spin_unlock_irqrestore(&priv->lock, flags);
>
>        hdr_len =3D ieee80211_get_hdrlen(fc);
>
>        /* Find (or create) index into station table for destination s=
tation */
>        sta_id =3D iwl4965_get_sta_id(priv, hdr);
> -       if (sta_id =3D=3D IWL_INVALID_STATION) {
> -               DECLARE_MAC_BUF(mac);
> -
> -               IWL_DEBUG_DROP("Dropping - INVALID STATION: %s\n",
> -                              print_mac(mac, hdr->addr1));
> -               goto drop;
> -       }
>
>        IWL_DEBUG_TX("station Id %d\n", sta_id);
>
>        qc =3D ieee80211_get_qos_ctrl(hdr);
>        if (qc) {
>                u8 tid =3D (u8)(le16_to_cpu(*qc) & 0xf);
>                seq_number =3D priv->stations[sta_id].tid[tid].seq_num=
ber &
>                                IEEE80211_SCTL_SEQ;
> @@ -5110,21 +5092,16 @@ static void iwl4965_mac_stop(struct ieee
>
>  static int iwl4965_mac_tx(struct ieee80211_hw *hw, struct sk_buff *s=
kb,
>                      struct ieee80211_tx_control *ctl)
>  {
>        struct iwl_priv *priv =3D hw->priv;
>
>        IWL_DEBUG_MAC80211("enter\n");
>
> -       if (priv->iw_mode =3D=3D IEEE80211_IF_TYPE_MNTR) {
> -               IWL_DEBUG_MAC80211("leave - monitor\n");
> -               return -1;
> -       }
> -
>        IWL_DEBUG_TX("dev->xmit(%d bytes) at rate 0x%02x\n", skb->len,
>                     ctl->tx_rate->bitrate);
>
>        if (iwl4965_tx_skb(priv, skb, ctl))
>                dev_kfree_skb_any(skb);
>
>        IWL_DEBUG_MAC80211("leave\n");
>        return 0;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wirel=
ess" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: iwlwifi injection bug

[Stefanik Gábor]

On Tue, May 20, 2008 at 9:41 AM, Tomas Winkler <tomasw@gmail.com> wrote=
:
> On Tue, May 20, 2008 at 4:14 PM, Stefanik G=E1bor <netrolller.3d@gmai=
l.com> wrote:
>> (Resend of previous mail, his time also sending to the list.)
>> On Tue, May 20, 2008 at 8:38 AM, Johannes Berg
>> <johannes@sipsolutions.net> wrote:
>>>
>>>> <snip>
>>>
>>> That code is completely bogus anyway IMO, it should just be removed=
=2E
>>>
>>>> <snip>
>>>
>>> Same here, why drop a frame if there's no station for it, that just
>>> means you can't do hw encryption which, well, cannot happen anyway =
since
>>> mac80211 would tell you about the station for hw crypto.
>>>
>>>> <snip>
>>>
>>> That's the code I was complaining about. Just kill it if it works t=
hen.
>>>
>>>> <snip>
>>>
>>> Same code, other driver. What are you doing poking with the 802.11
>>> frames you're supposed to transmit anyway?
>>>
>>> I wish you guys at Intel would trust mac80211 and not try to work a=
round
>>> it all the time.
>>>
>>> johannes
>>>
>>
>> Patch with nits addressed, a syslog issue fixed and also with a
>> bracketing fix in 3945:
>>
>> <snip>
> This seams Okay.
>> <snip>
>
> Not sure about this
>
>> <snip>
> The thing is that mac80211 and uCode are not exactly in sync on
> association time.
> I'm not sure but this would be also spec violation.
>
>> <snip>
> send packet with invalid station will certainly crash uCode.
>
>> <snip>
>

Thanks for the feedback, here is a fixed patch:

diff -rp -U 8 compat-wireless-2008-05-20.orig/drivers/net/wireless/iwlw=
ifi/iwl3945-base.c
compat-wireless-2008-05-20/drivers/net/wireless/iwlwifi/iwl3945-base.c
--- compat-wireless-2008-05-20.orig/drivers/net/wireless/iwlwifi/iwl394=
5-base.c	2008-05-20
05:05:29.000000000 -0400
+++ compat-wireless-2008-05-20/drivers/net/wireless/iwlwifi/iwl3945-bas=
e.c	2008-05-20
10:11:58.809424284 -0400
@@ -2542,16 +2542,19 @@ static int iwl3945_get_sta_id(struct iwl
 			return sta_id;

 		IWL_DEBUG_DROP("Station %s not in station map. "
 			       "Defaulting to broadcast...\n",
 			       print_mac(mac, hdr->addr1));
 		iwl3945_print_hex_dump(IWL_DL_DROP, (u8 *) hdr, sizeof(*hdr));
 		return priv->hw_setting.bcast_sta_id;
 	}
+	/* If we are in monitor mode, use BCAST */
+	case IEEE80211_IF_TYPE_MNTR:
+		return priv->hw_setting.bcast_sta_id;
 	default:
 		IWL_WARNING("Unknown mode of operation: %d", priv->iw_mode);
 		return priv->hw_setting.bcast_sta_id;
 	}
 }

 /*
  * start REPLY_TX command process
@@ -2579,18 +2582,18 @@ static int iwl3945_tx_skb(struct iwl3945
 	int rc;

 	spin_lock_irqsave(&priv->lock, flags);
 	if (iwl3945_is_rfkill(priv)) {
 		IWL_DEBUG_DROP("Dropping - RF KILL\n");
 		goto drop_unlock;
 	}

-	if (!priv->vif) {
-		IWL_DEBUG_DROP("Dropping - !priv->vif\n");
+	if (!priv->vif && priv->iw_mode !=3D IEEE80211_IF_TYPE_MNTR) {
+		IWL_DEBUG_DROP("Dropping - !priv->vif outside monitor mode\n");
 		goto drop_unlock;
 	}

 	if ((ctl->tx_rate->hw_value & 0xFF) =3D=3D IWL_INVALID_RATE) {
 		IWL_ERROR("ERROR: No TX rate available.\n");
 		goto drop_unlock;
 	}

@@ -2603,36 +2606,35 @@ static int iwl3945_tx_skb(struct iwl3945
 	if (ieee80211_is_auth(fc))
 		IWL_DEBUG_TX("Sending AUTH frame\n");
 	else if (ieee80211_is_assoc_request(fc))
 		IWL_DEBUG_TX("Sending ASSOC frame\n");
 	else if (ieee80211_is_reassoc_request(fc))
 		IWL_DEBUG_TX("Sending REASSOC frame\n");
 #endif

-	/* drop all data frame if we are not associated */
-	if ((!iwl3945_is_associated(priv) ||
+	/* drop all data frame if we are not associated and not monitoring */
+	if (((!iwl3945_is_associated(priv) && priv->iw_mode !=3D
IEEE80211_IF_TYPE_MNTR) ||
 	     ((priv->iw_mode =3D=3D IEEE80211_IF_TYPE_STA) && !priv->assoc_id=
)) &&
 	    ((fc & IEEE80211_FCTL_FTYPE) =3D=3D IEEE80211_FTYPE_DATA)) {
-		IWL_DEBUG_DROP("Dropping - !iwl3945_is_associated\n");
+		IWL_DEBUG_DROP("Dropping - !iwl3945_is_associated outside monitor mo=
de\n");
 		goto drop_unlock;
 	}

 	spin_unlock_irqrestore(&priv->lock, flags);

 	hdr_len =3D ieee80211_get_hdrlen(fc);

 	/* Find (or create) index into station table for destination station =
*/
 	sta_id =3D iwl3945_get_sta_id(priv, hdr);
+
+	/* If the station is invalid, assume broadcast */
 	if (sta_id =3D=3D IWL_INVALID_STATION) {
 		DECLARE_MAC_BUF(mac);
-
-		IWL_DEBUG_DROP("Dropping - INVALID STATION: %s\n",
-			       print_mac(mac, hdr->addr1));
-		goto drop;
+		sta_id =3D priv->hw_setting.bcast_sta_id;
 	}

 	IWL_DEBUG_RATE("station Id %d\n", sta_id);

 	qc =3D ieee80211_get_qos_ctrl(hdr);
 	if (qc) {
 		u8 tid =3D (u8)(le16_to_cpu(*qc) & 0xf);
 		seq_number =3D priv->stations[sta_id].tid[tid].seq_number &
@@ -6692,21 +6694,16 @@ static void iwl3945_mac_stop(struct ieee

 static int iwl3945_mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb=
,
 		      struct ieee80211_tx_control *ctl)
 {
 	struct iwl3945_priv *priv =3D hw->priv;

 	IWL_DEBUG_MAC80211("enter\n");

-	if (priv->iw_mode =3D=3D IEEE80211_IF_TYPE_MNTR) {
-		IWL_DEBUG_MAC80211("leave - monitor\n");
-		return -1;
-	}
-
 	IWL_DEBUG_TX("dev->xmit(%d bytes) at rate 0x%02x\n", skb->len,
 		     ctl->tx_rate->bitrate);

 	if (iwl3945_tx_skb(priv, skb, ctl))
 		dev_kfree_skb_any(skb);

 	IWL_DEBUG_MAC80211("leave\n");
 	return 0;
diff -rp -U 8 compat-wireless-2008-05-20.orig/drivers/net/wireless/iwlw=
ifi/iwl4965-base.c
compat-wireless-2008-05-20/drivers/net/wireless/iwlwifi/iwl4965-base.c
--- compat-wireless-2008-05-20.orig/drivers/net/wireless/iwlwifi/iwl496=
5-base.c	2008-05-20
05:05:29.000000000 -0400
+++ compat-wireless-2008-05-20/drivers/net/wireless/iwlwifi/iwl4965-bas=
e.c	2008-05-20
10:07:02.644103448 -0400
@@ -1746,16 +1746,19 @@ static int iwl4965_get_sta_id(struct iwl
 			return sta_id;

 		IWL_DEBUG_DROP("Station %s not in station map. "
 			       "Defaulting to broadcast...\n",
 			       print_mac(mac, hdr->addr1));
 		iwl_print_hex_dump(priv, IWL_DL_DROP, (u8 *) hdr, sizeof(*hdr));
 		return priv->hw_params.bcast_sta_id;

+	/* If we are in monitor mode, use BCAST */
+	case IEEE80211_IF_TYPE_MNTR:
+		return priv->hw_params.bcast_sta_id;
 	default:
 		IWL_WARNING("Unknown mode of operation: %d", priv->iw_mode);
 		return priv->hw_params.bcast_sta_id;
 	}
 }

 /*
  * start REPLY_TX command process
@@ -1784,18 +1787,18 @@ static int iwl4965_tx_skb(struct iwl_pri
 	int rc;

 	spin_lock_irqsave(&priv->lock, flags);
 	if (iwl_is_rfkill(priv)) {
 		IWL_DEBUG_DROP("Dropping - RF KILL\n");
 		goto drop_unlock;
 	}

-	if (!priv->vif) {
-		IWL_DEBUG_DROP("Dropping - !priv->vif\n");
+	if (!priv->vif && priv->iw_mode !=3D IEEE80211_IF_TYPE_MNTR) {
+		IWL_DEBUG_DROP("Dropping - !priv->vif outside monitor mode\n");
 		goto drop_unlock;
 	}

 	if ((ctl->tx_rate->hw_value & 0xFF) =3D=3D IWL_INVALID_RATE) {
 		IWL_ERROR("ERROR: No TX rate available.\n");
 		goto drop_unlock;
 	}

@@ -1808,36 +1811,36 @@ static int iwl4965_tx_skb(struct iwl_pri
 	if (ieee80211_is_auth(fc))
 		IWL_DEBUG_TX("Sending AUTH frame\n");
 	else if (ieee80211_is_assoc_request(fc))
 		IWL_DEBUG_TX("Sending ASSOC frame\n");
 	else if (ieee80211_is_reassoc_request(fc))
 		IWL_DEBUG_TX("Sending REASSOC frame\n");
 #endif

-	/* drop all data frame if we are not associated */
+	/* drop all data frame if we are not associated and not monitoring */
 	if (((fc & IEEE80211_FCTL_FTYPE) =3D=3D IEEE80211_FTYPE_DATA) &&
-	   (!iwl_is_associated(priv) ||
+	   ((!iwl_is_associated(priv) && priv->iw_mode !=3D IEEE80211_IF_TYPE=
_MNTR) ||
 	    ((priv->iw_mode =3D=3D IEEE80211_IF_TYPE_STA) && !priv->assoc_id)=
 ||
 	    !priv->assoc_station_added)) {
-		IWL_DEBUG_DROP("Dropping - !iwl_is_associated\n");
+		IWL_DEBUG_DROP("Dropping - !iwl_is_associated outside monitor mode\n=
");
 		goto drop_unlock;
 	}

 	spin_unlock_irqrestore(&priv->lock, flags);

 	hdr_len =3D ieee80211_get_hdrlen(fc);

 	/* Find (or create) index into station table for destination station =
*/
 	sta_id =3D iwl4965_get_sta_id(priv, hdr);
+
+	/* If the station is invalid, assume broadcast */
 	if (sta_id =3D=3D IWL_INVALID_STATION) {
 		DECLARE_MAC_BUF(mac);
-
-		IWL_DEBUG_DROP("Dropping - INVALID STATION: %s\n",
-			       print_mac(mac, hdr->addr1));
+		sta_id =3D priv->hw_params.bcast_sta_id;
 		goto drop;
 	}

 	IWL_DEBUG_TX("station Id %d\n", sta_id);

 	qc =3D ieee80211_get_qos_ctrl(hdr);
 	if (qc) {
 		u8 tid =3D (u8)(le16_to_cpu(*qc) & 0xf);
@@ -5110,21 +5113,16 @@ static void iwl4965_mac_stop(struct ieee

 static int iwl4965_mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb=
,
 		      struct ieee80211_tx_control *ctl)
 {
 	struct iwl_priv *priv =3D hw->priv;

 	IWL_DEBUG_MAC80211("enter\n");

-	if (priv->iw_mode =3D=3D IEEE80211_IF_TYPE_MNTR) {
-		IWL_DEBUG_MAC80211("leave - monitor\n");
-		return -1;
-	}
-
 	IWL_DEBUG_TX("dev->xmit(%d bytes) at rate 0x%02x\n", skb->len,
 		     ctl->tx_rate->bitrate);

 	if (iwl4965_tx_skb(priv, skb, ctl))
 		dev_kfree_skb_any(skb);

 	IWL_DEBUG_MAC80211("leave\n");
 	return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: iwlwifi injection bug

[Stefanik Gábor]

BTW I am not sure if the "if (sta_id == IWL_INVALID_STATION)" change
is needed at all - injected packets should have a valid station ID,
due to the "case IEEE80211_IF_TYPE_MNTR" change.

Re: iwlwifi injection bug

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 873 bytes --]


> -	/* drop all data frame if we are not associated */
> -	if ((!iwl3945_is_associated(priv) ||
> +	/* drop all data frame if we are not associated and not monitoring */
> +	if (((!iwl3945_is_associated(priv) && priv->iw_mode !=
> IEEE80211_IF_TYPE_MNTR) ||
>  	     ((priv->iw_mode == IEEE80211_IF_TYPE_STA) && !priv->assoc_id)) &&
>  	    ((fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_DATA)) {
> -		IWL_DEBUG_DROP("Dropping - !iwl3945_is_associated\n");
> +		IWL_DEBUG_DROP("Dropping - !iwl3945_is_associated outside monitor mode\n");
>  		goto drop_unlock;
>  	}

This makes injection not work when you're not exclusively in monitor
mode. Yes, it may break the spec to be sending frames to an arbitrary
station, but it's useful for all kinds of things including testing to
have a monitor and a regular interface working at the same time.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: iwlwifi injection bug

[Stefanik Gábor]

On Tue, May 20, 2008 at 10:27 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
>
>> -     /* drop all data frame if we are not associated */
>> -     if ((!iwl3945_is_associated(priv) ||
>> +     /* drop all data frame if we are not associated and not monitoring */
>> +     if (((!iwl3945_is_associated(priv) && priv->iw_mode !=
>> IEEE80211_IF_TYPE_MNTR) ||
>>            ((priv->iw_mode == IEEE80211_IF_TYPE_STA) && !priv->assoc_id)) &&
>>           ((fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_DATA)) {
>> -             IWL_DEBUG_DROP("Dropping - !iwl3945_is_associated\n");
>> +             IWL_DEBUG_DROP("Dropping - !iwl3945_is_associated outside monitor mode\n");
>>               goto drop_unlock;
>>       }
>
> This makes injection not work when you're not exclusively in monitor
> mode. Yes, it may break the spec to be sending frames to an arbitrary
> station, but it's useful for all kinds of things including testing to
> have a monitor and a regular interface working at the same time.
>
> johannes
>

What do you think is a better way to check if we have a monitor
interface? && priv->iw_mode == IEEE80211_IF_TYPE_STA? Or should this
be removed altogether?

-- 
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

Re: iwlwifi injection bug

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1482 bytes --]

On Tue, 2008-05-20 at 10:58 -0400, Stefanik Gábor wrote:
> On Tue, May 20, 2008 at 10:27 AM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> >
> >> -     /* drop all data frame if we are not associated */
> >> -     if ((!iwl3945_is_associated(priv) ||
> >> +     /* drop all data frame if we are not associated and not monitoring */
> >> +     if (((!iwl3945_is_associated(priv) && priv->iw_mode !=
> >> IEEE80211_IF_TYPE_MNTR) ||
> >>            ((priv->iw_mode == IEEE80211_IF_TYPE_STA) && !priv->assoc_id)) &&
> >>           ((fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_DATA)) {
> >> -             IWL_DEBUG_DROP("Dropping - !iwl3945_is_associated\n");
> >> +             IWL_DEBUG_DROP("Dropping - !iwl3945_is_associated outside monitor mode\n");
> >>               goto drop_unlock;
> >>       }
> >
> > This makes injection not work when you're not exclusively in monitor
> > mode. Yes, it may break the spec to be sending frames to an arbitrary
> > station, but it's useful for all kinds of things including testing to
> > have a monitor and a regular interface working at the same time.
> >
> > johannes
> >
> 
> What do you think is a better way to check if we have a monitor
> interface? && priv->iw_mode == IEEE80211_IF_TYPE_STA? Or should this
> be removed altogether?

I tend to think it should be removed, in regular operation mac80211
would never create such a data frame, and when injecting you actually
want it.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: iwlwifi injection bug

[Stefanik Gábor]

On Tue, May 20, 2008 at 11:18 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> I tend to think it should be removed, in regular operation mac80211
> would never create such a data frame, and when injecting you actually
> want it.
>
> johannes
>

OK! I have also determined that my IWL_INVALID_STATION change is
unnecessary, as injected packets do have a valid station ID (they are
marked as broadcast by iwl*9*5_get_sta_id). The IWL_INVALID_STATION
change was a hack I put in earlier, before finding the real problems
in iwl*9*5_get_sta_id and iwl*9*5_mac_tx.

----

diff -rp -U 8 compat-wireless-2008-05-20.orig/drivers/net/wireless/iwlwifi/iwl3945-base.c
compat-wireless-2008-05-20/drivers/net/wireless/iwlwifi/iwl3945-base.c
--- compat-wireless-2008-05-20.orig/drivers/net/wireless/iwlwifi/iwl3945-base.c	2008-05-20
05:05:29.000000000 -0400
+++ compat-wireless-2008-05-20/drivers/net/wireless/iwlwifi/iwl3945-base.c	2008-05-20
11:31:15.513173847 -0400
@@ -2542,16 +2542,19 @@ static int iwl3945_get_sta_id(struct iwl
 			return sta_id;

 		IWL_DEBUG_DROP("Station %s not in station map. "
 			       "Defaulting to broadcast...\n",
 			       print_mac(mac, hdr->addr1));
 		iwl3945_print_hex_dump(IWL_DL_DROP, (u8 *) hdr, sizeof(*hdr));
 		return priv->hw_setting.bcast_sta_id;
 	}
+	/* If we are in monitor mode, use BCAST */
+	case IEEE80211_IF_TYPE_MNTR:
+		return priv->hw_setting.bcast_sta_id;
 	default:
 		IWL_WARNING("Unknown mode of operation: %d", priv->iw_mode);
 		return priv->hw_setting.bcast_sta_id;
 	}
 }

 /*
  * start REPLY_TX command process
@@ -2579,21 +2582,16 @@ static int iwl3945_tx_skb(struct iwl3945
 	int rc;

 	spin_lock_irqsave(&priv->lock, flags);
 	if (iwl3945_is_rfkill(priv)) {
 		IWL_DEBUG_DROP("Dropping - RF KILL\n");
 		goto drop_unlock;
 	}

-	if (!priv->vif) {
-		IWL_DEBUG_DROP("Dropping - !priv->vif\n");
-		goto drop_unlock;
-	}
-
 	if ((ctl->tx_rate->hw_value & 0xFF) == IWL_INVALID_RATE) {
 		IWL_ERROR("ERROR: No TX rate available.\n");
 		goto drop_unlock;
 	}

 	unicast = !is_multicast_ether_addr(hdr->addr1);
 	id = 0;

@@ -2603,24 +2601,16 @@ static int iwl3945_tx_skb(struct iwl3945
 	if (ieee80211_is_auth(fc))
 		IWL_DEBUG_TX("Sending AUTH frame\n");
 	else if (ieee80211_is_assoc_request(fc))
 		IWL_DEBUG_TX("Sending ASSOC frame\n");
 	else if (ieee80211_is_reassoc_request(fc))
 		IWL_DEBUG_TX("Sending REASSOC frame\n");
 #endif

-	/* drop all data frame if we are not associated */
-	if ((!iwl3945_is_associated(priv) ||
-	     ((priv->iw_mode == IEEE80211_IF_TYPE_STA) && !priv->assoc_id)) &&
-	    ((fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_DATA)) {
-		IWL_DEBUG_DROP("Dropping - !iwl3945_is_associated\n");
-		goto drop_unlock;
-	}
-
 	spin_unlock_irqrestore(&priv->lock, flags);

 	hdr_len = ieee80211_get_hdrlen(fc);

 	/* Find (or create) index into station table for destination station */
 	sta_id = iwl3945_get_sta_id(priv, hdr);
 	if (sta_id == IWL_INVALID_STATION) {
 		DECLARE_MAC_BUF(mac);
@@ -6692,21 +6682,16 @@ static void iwl3945_mac_stop(struct ieee

 static int iwl3945_mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb,
 		      struct ieee80211_tx_control *ctl)
 {
 	struct iwl3945_priv *priv = hw->priv;

 	IWL_DEBUG_MAC80211("enter\n");

-	if (priv->iw_mode == IEEE80211_IF_TYPE_MNTR) {
-		IWL_DEBUG_MAC80211("leave - monitor\n");
-		return -1;
-	}
-
 	IWL_DEBUG_TX("dev->xmit(%d bytes) at rate 0x%02x\n", skb->len,
 		     ctl->tx_rate->bitrate);

 	if (iwl3945_tx_skb(priv, skb, ctl))
 		dev_kfree_skb_any(skb);

 	IWL_DEBUG_MAC80211("leave\n");
 	return 0;
diff -rp -U 8 compat-wireless-2008-05-20.orig/drivers/net/wireless/iwlwifi/iwl4965-base.c
compat-wireless-2008-05-20/drivers/net/wireless/iwlwifi/iwl4965-base.c
--- compat-wireless-2008-05-20.orig/drivers/net/wireless/iwlwifi/iwl4965-base.c	2008-05-20
05:05:29.000000000 -0400
+++ compat-wireless-2008-05-20/drivers/net/wireless/iwlwifi/iwl4965-base.c	2008-05-20
11:31:54.278205087 -0400
@@ -1746,16 +1746,19 @@ static int iwl4965_get_sta_id(struct iwl
 			return sta_id;

 		IWL_DEBUG_DROP("Station %s not in station map. "
 			       "Defaulting to broadcast...\n",
 			       print_mac(mac, hdr->addr1));
 		iwl_print_hex_dump(priv, IWL_DL_DROP, (u8 *) hdr, sizeof(*hdr));
 		return priv->hw_params.bcast_sta_id;

+	/* If we are in monitor mode, use BCAST */
+	case IEEE80211_IF_TYPE_MNTR:
+		return priv->hw_params.bcast_sta_id;
 	default:
 		IWL_WARNING("Unknown mode of operation: %d", priv->iw_mode);
 		return priv->hw_params.bcast_sta_id;
 	}
 }

 /*
  * start REPLY_TX command process
@@ -1784,21 +1787,16 @@ static int iwl4965_tx_skb(struct iwl_pri
 	int rc;

 	spin_lock_irqsave(&priv->lock, flags);
 	if (iwl_is_rfkill(priv)) {
 		IWL_DEBUG_DROP("Dropping - RF KILL\n");
 		goto drop_unlock;
 	}

-	if (!priv->vif) {
-		IWL_DEBUG_DROP("Dropping - !priv->vif\n");
-		goto drop_unlock;
-	}
-
 	if ((ctl->tx_rate->hw_value & 0xFF) == IWL_INVALID_RATE) {
 		IWL_ERROR("ERROR: No TX rate available.\n");
 		goto drop_unlock;
 	}

 	unicast = !is_multicast_ether_addr(hdr->addr1);
 	id = 0;

@@ -1808,25 +1806,16 @@ static int iwl4965_tx_skb(struct iwl_pri
 	if (ieee80211_is_auth(fc))
 		IWL_DEBUG_TX("Sending AUTH frame\n");
 	else if (ieee80211_is_assoc_request(fc))
 		IWL_DEBUG_TX("Sending ASSOC frame\n");
 	else if (ieee80211_is_reassoc_request(fc))
 		IWL_DEBUG_TX("Sending REASSOC frame\n");
 #endif

-	/* drop all data frame if we are not associated */
-	if (((fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_DATA) &&
-	   (!iwl_is_associated(priv) ||
-	    ((priv->iw_mode == IEEE80211_IF_TYPE_STA) && !priv->assoc_id) ||
-	    !priv->assoc_station_added)) {
-		IWL_DEBUG_DROP("Dropping - !iwl_is_associated\n");
-		goto drop_unlock;
-	}
-
 	spin_unlock_irqrestore(&priv->lock, flags);

 	hdr_len = ieee80211_get_hdrlen(fc);

 	/* Find (or create) index into station table for destination station */
 	sta_id = iwl4965_get_sta_id(priv, hdr);
 	if (sta_id == IWL_INVALID_STATION) {
 		DECLARE_MAC_BUF(mac);
@@ -5110,21 +5099,16 @@ static void iwl4965_mac_stop(struct ieee

 static int iwl4965_mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb,
 		      struct ieee80211_tx_control *ctl)
 {
 	struct iwl_priv *priv = hw->priv;

 	IWL_DEBUG_MAC80211("enter\n");

-	if (priv->iw_mode == IEEE80211_IF_TYPE_MNTR) {
-		IWL_DEBUG_MAC80211("leave - monitor\n");
-		return -1;
-	}
-
 	IWL_DEBUG_TX("dev->xmit(%d bytes) at rate 0x%02x\n", skb->len,
 		     ctl->tx_rate->bitrate);

 	if (iwl4965_tx_skb(priv, skb, ctl))
 		dev_kfree_skb_any(skb);

 	IWL_DEBUG_MAC80211("leave\n");
 	return 0;

Re: iwlwifi injection bug

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 438 bytes --]


> OK! I have also determined that my IWL_INVALID_STATION change is
> unnecessary, as injected packets do have a valid station ID (they are
> marked as broadcast by iwl*9*5_get_sta_id). The IWL_INVALID_STATION
> change was a hack I put in earlier, before finding the real problems
> in iwl*9*5_get_sta_id and iwl*9*5_mac_tx.

Now you only need to read Documentation/SubmittingPatches and not
linewrap your patches ;)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]