2012-12-11 17:27:40

by Gabor Juhos

[permalink] [raw]
Subject: [PATCH v2] rt2x00: zero-out rx_status

In commit 'mac80211: support radiotap vendor namespace RX data'
new fields were added to 'struct ieee80211_rx_status' and those
fileds must be zeroed. However the rt2x00 driver stores driver
specific data in the cb array of the rx skbs, so the fields
might contain garbage and this can cause unexpected behaviour.

The rt2x00 driver from the compat-wireless-2012-12-01
tarball caused the following warning:

WARNING: at
/devel/ramips/build_dir/target-mipsel_r2_uClibc-0.9.33.2/linux-ramips_rt305x/
compat-wireless-2012-12-01/net/mac80211/rx.c:115 ieee80211_rx_irqsafe+0x274/0xbcc
[mac80211]()
Modules linked in: dwc_otg ledtrig_usbdev nf_nat_irc
nf_nat_ftp nf_conntrack_irc nf_conntrack_ftp ipt_MASQUERADE
iptable_nat nf_nat pppoe xt_conntrack xt_CT xt_NOTRACK iptable_raw
xt_state nf_conntrack_ipv4 nf_defrag_ipv4 nf_conntrack pppox
ipt_REJECT xt_TCPMSS xt_comment xt_multiport xt_mac xt_limit
iptable_mangle iptable_filter ip_tables xt_tcpudp x_tables ppp_async
ppp_generic slhc rt2800pci(O) rt2800lib(O) rt2x00soc(O) rt2x00pci(O)
rt2x00lib(O) mac80211(O) usbcore usb_common nls_base crc_itu_t
crc_ccitt eeprom_93cx6 cfg80211(O) compat(O) arc4 aes_generic
crypto_blkcipher cryptomgr aead crypto_hash crypto_algapi leds_gpio
button_hotplug(O) gpio_keys_polled input_polldev input_core
Call Trace:
[<801e96b4>] dump_stack+0x8/0x34
[<80010a9c>] warn_slowpath_common+0x78/0xa4
[<80010ae0>] warn_slowpath_null+0x18/0x24
[<80a9710c>] ieee80211_rx_irqsafe+0x274/0xbcc [mac80211]

The patch ensures that each field gets initialized with
zeroes.

Cc: <[email protected]>
Signed-off-by: Gabor Juhos <[email protected]>
---
v2:
- update the commit message and add a comment to the code
- drop the ath5k and p54 patches
---
drivers/net/wireless/rt2x00/rt2x00dev.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
index 3248b42..507c8c5 100644
--- a/drivers/net/wireless/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
@@ -686,6 +686,14 @@ void rt2x00lib_rxdone(struct queue_entry *entry, gfp_t gfp)
* to mac80211.
*/
rx_status = IEEE80211_SKB_RXCB(entry->skb);
+
+ /* Ensure that all fields of rx_status are initialized
+ * properly. The skb->cb array was used for driver
+ * specific informations, so rx_status might contain
+ * garbage.
+ */
+ memset(rx_status, 0, sizeof(*rx_status));
+
rx_status->mactime = rxdesc.timestamp;
rx_status->band = rt2x00dev->curr_band;
rx_status->freq = rt2x00dev->curr_freq;
--
1.7.10



2012-12-11 21:23:58

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH v2] rt2x00: zero-out rx_status



Sent from my iPad

On 11 dec. 2012, at 18:27, Gabor Juhos <[email protected]> wrote:

> In commit 'mac80211: support radiotap vendor namespace RX data'
> new fields were added to 'struct ieee80211_rx_status' and those
> fileds must be zeroed. However the rt2x00 driver stores driver
> specific data in the cb array of the rx skbs, so the fields
> might contain garbage and this can cause unexpected behaviour.
>
> The rt2x00 driver from the compat-wireless-2012-12-01
> tarball caused the following warning:
>
> WARNING: at
> /devel/ramips/build_dir/target-mipsel_r2_uClibc-0.9.33.2/linux-ramips_rt305x/
> compat-wireless-2012-12-01/net/mac80211/rx.c:115 ieee80211_rx_irqsafe+0x274/0xbcc
> [mac80211]()
> Modules linked in: dwc_otg ledtrig_usbdev nf_nat_irc
> nf_nat_ftp nf_conntrack_irc nf_conntrack_ftp ipt_MASQUERADE
> iptable_nat nf_nat pppoe xt_conntrack xt_CT xt_NOTRACK iptable_raw
> xt_state nf_conntrack_ipv4 nf_defrag_ipv4 nf_conntrack pppox
> ipt_REJECT xt_TCPMSS xt_comment xt_multiport xt_mac xt_limit
> iptable_mangle iptable_filter ip_tables xt_tcpudp x_tables ppp_async
> ppp_generic slhc rt2800pci(O) rt2800lib(O) rt2x00soc(O) rt2x00pci(O)
> rt2x00lib(O) mac80211(O) usbcore usb_common nls_base crc_itu_t
> crc_ccitt eeprom_93cx6 cfg80211(O) compat(O) arc4 aes_generic
> crypto_blkcipher cryptomgr aead crypto_hash crypto_algapi leds_gpio
> button_hotplug(O) gpio_keys_polled input_polldev input_core
> Call Trace:
> [<801e96b4>] dump_stack+0x8/0x34
> [<80010a9c>] warn_slowpath_common+0x78/0xa4
> [<80010ae0>] warn_slowpath_null+0x18/0x24
> [<80a9710c>] ieee80211_rx_irqsafe+0x274/0xbcc [mac80211]
>
> The patch ensures that each field gets initialized with
> zeroes.
>
> Cc: <[email protected]>
> Signed-off-by: Gabor Juhos <[email protected]>

Acked-by: Gertjan van Wingerde <[email protected]>

> ---
> v2:
> - update the commit message and add a comment to the code
> - drop the ath5k and p54 patches
> ---
> drivers/net/wireless/rt2x00/rt2x00dev.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
> index 3248b42..507c8c5 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00dev.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
> @@ -686,6 +686,14 @@ void rt2x00lib_rxdone(struct queue_entry *entry, gfp_t gfp)
> * to mac80211.
> */
> rx_status = IEEE80211_SKB_RXCB(entry->skb);
> +
> + /* Ensure that all fields of rx_status are initialized
> + * properly. The skb->cb array was used for driver
> + * specific informations, so rx_status might contain
> + * garbage.
> + */
> + memset(rx_status, 0, sizeof(*rx_status));
> +
> rx_status->mactime = rxdesc.timestamp;
> rx_status->band = rt2x00dev->curr_band;
> rx_status->freq = rt2x00dev->curr_freq;
> --
> 1.7.10
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html