2010-06-09 10:43:52

by Juuso Oikarinen

[permalink] [raw]
Subject: [PATCHv4] mac80211: Fix circular locking dependency in ARP filter handling

There is a circular locking dependency when configuring the
hardware ARP filters on association, occurring when flushing the mac80211
workqueue. This is what happens:

[ 92.026800] =======================================================
[ 92.030507] [ INFO: possible circular locking dependency detected ]
[ 92.030507] 2.6.34-04781-g2b2c009 #85
[ 92.030507] -------------------------------------------------------
[ 92.030507] modprobe/5225 is trying to acquire lock:
[ 92.030507] ((wiphy_name(local->hw.wiphy))){+.+.+.}, at: [<ffffffff8105b5c0>] flush_workq
ueue+0x0/0xb0
[ 92.030507]
[ 92.030507] but task is already holding lock:
[ 92.030507] (rtnl_mutex){+.+.+.}, at: [<ffffffff812b9ce2>] rtnl_lock+0x12/0x20
[ 92.030507]
[ 92.030507] which lock already depends on the new lock.
[ 92.030507]
[ 92.030507]
[ 92.030507] the existing dependency chain (in reverse order) is:
[ 92.030507]
[ 92.030507] -> #2 (rtnl_mutex){+.+.+.}:
[ 92.030507] [<ffffffff810761fb>] lock_acquire+0xdb/0x110
[ 92.030507] [<ffffffff81341754>] mutex_lock_nested+0x44/0x300
[ 92.030507] [<ffffffff812b9ce2>] rtnl_lock+0x12/0x20
[ 92.030507] [<ffffffffa022d47c>] ieee80211_assoc_done+0x6c/0xe0 [mac80211]
[ 92.030507] [<ffffffffa022f2ad>] ieee80211_work_work+0x31d/0x1280 [mac80211]

[ 92.030507] -> #1 ((&local->work_work)){+.+.+.}:
[ 92.030507] [<ffffffff810761fb>] lock_acquire+0xdb/0x110
[ 92.030507] [<ffffffff8105a51a>] worker_thread+0x22a/0x370
[ 92.030507] [<ffffffff8105ecc6>] kthread+0x96/0xb0
[ 92.030507] [<ffffffff81003a94>] kernel_thread_helper+0x4/0x10
[ 92.030507]
[ 92.030507] -> #0 ((wiphy_name(local->hw.wiphy))){+.+.+.}:
[ 92.030507] [<ffffffff81075fdc>] __lock_acquire+0x1c0c/0x1d50
[ 92.030507] [<ffffffff810761fb>] lock_acquire+0xdb/0x110
[ 92.030507] [<ffffffff8105b60e>] flush_workqueue+0x4e/0xb0
[ 92.030507] [<ffffffffa023ff7b>] ieee80211_stop_device+0x2b/0xb0 [mac80211]
[ 92.030507] [<ffffffffa0231635>] ieee80211_stop+0x3e5/0x680 [mac80211]

The locking in this case is quite complex. Fix the problem by rewriting the
way the hardware ARP filter list is handled - i.e. make a copy of the address
list to the bss_conf struct, and provide that list to the hardware driver
when needed.

The current patch will enable filtering also in promiscuous mode. This may need
to be changed in the future.

Reported-by: Reinette Chatre <[email protected]>
Signed-off-by: Juuso Oikarinen <[email protected]>
---
v4: Remove the problematic promiscuous mode awareness

include/net/mac80211.h | 35 ++++++++++++++++-----------
net/mac80211/driver-ops.h | 17 -------------
net/mac80211/driver-trace.h | 25 --------------------
net/mac80211/ieee80211_i.h | 2 +
net/mac80211/iface.c | 3 ++
net/mac80211/main.c | 54 +++++++++++++++++++++++++++----------------
net/mac80211/mlme.c | 35 +++++++++++++++------------
7 files changed, 79 insertions(+), 92 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index e3c1d47..09c4c89 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -19,7 +19,6 @@
#include <linux/wireless.h>
#include <linux/device.h>
#include <linux/ieee80211.h>
-#include <linux/inetdevice.h>
#include <net/cfg80211.h>

/**
@@ -147,6 +146,7 @@ struct ieee80211_low_level_stats {
* enabled/disabled (beaconing modes)
* @BSS_CHANGED_CQM: Connection quality monitor config changed
* @BSS_CHANGED_IBSS: IBSS join status changed
+ * @BSS_CHANGED_ARP_FILTER: Hardware ARP filter address list or state changed.
*/
enum ieee80211_bss_change {
BSS_CHANGED_ASSOC = 1<<0,
@@ -161,10 +161,18 @@ enum ieee80211_bss_change {
BSS_CHANGED_BEACON_ENABLED = 1<<9,
BSS_CHANGED_CQM = 1<<10,
BSS_CHANGED_IBSS = 1<<11,
+ BSS_CHANGED_ARP_FILTER = 1<<12,

/* when adding here, make sure to change ieee80211_reconfig */
};

+/*
+ * The maximum number of IPv4 addresses listed for ARP filtering. If the number
+ * of addresses for an interface increase beyond this value, hardware ARP
+ * filtering will be disabled.
+ */
+#define IEEE80211_BSS_ARP_ADDR_LIST_LEN 4
+
/**
* struct ieee80211_bss_conf - holds the BSS's changing parameters
*
@@ -200,6 +208,15 @@ enum ieee80211_bss_change {
* @cqm_rssi_thold: Connection quality monitor RSSI threshold, a zero value
* implies disabled
* @cqm_rssi_hyst: Connection quality monitor RSSI hysteresis
+ * @arp_addr_list: List of IPv4 addresses for hardware ARP filtering. The
+ * may filter ARP queries targeted for other addresses than listed here.
+ * The driver must allow ARP queries targeted for all address listed here
+ * to pass through. An empty list implies no ARP queries need to pass.
+ * @arp_addr_cnt: Number of addresses currently on the list.
+ * @arp_filter_enabled: Enable ARP filtering - if enabled, the hardware may
+ * filter ARP queries based on the @arp_addr_list, if disabled, the
+ * hardware must not perform any ARP filtering. Note, that the filter will
+ * be enabled also in promiscuous mode.
*/
struct ieee80211_bss_conf {
const u8 *bssid;
@@ -220,6 +237,9 @@ struct ieee80211_bss_conf {
s32 cqm_rssi_thold;
u32 cqm_rssi_hyst;
enum nl80211_channel_type channel_type;
+ __be32 arp_addr_list[IEEE80211_BSS_ARP_ADDR_LIST_LEN];
+ u8 arp_addr_cnt;
+ bool arp_filter_enabled;
};

/**
@@ -1529,16 +1549,6 @@ enum ieee80211_ampdu_mlme_action {
* of the bss parameters has changed when a call is made. The callback
* can sleep.
*
- * @configure_arp_filter: Configuration function for hardware ARP query filter.
- * This function is called with all the IP addresses configured to the
- * interface as argument - all ARP queries targeted to any of these
- * addresses must pass through. If the hardware filter does not support
- * enought addresses, hardware filtering must be disabled. The ifa_list
- * argument may be NULL, indicating that filtering must be disabled.
- * This function is called upon association complete with current
- * address(es), and while associated whenever the IP address(es) change.
- * The callback can sleep.
- *
* @prepare_multicast: Prepare for multicast filter configuration.
* This callback is optional, and its return value is passed
* to configure_filter(). This callback must be atomic.
@@ -1678,9 +1688,6 @@ struct ieee80211_ops {
struct ieee80211_vif *vif,
struct ieee80211_bss_conf *info,
u32 changed);
- int (*configure_arp_filter)(struct ieee80211_hw *hw,
- struct ieee80211_vif *vif,
- struct in_ifaddr *ifa_list);
u64 (*prepare_multicast)(struct ieee80211_hw *hw,
struct netdev_hw_addr_list *mc_list);
void (*configure_filter)(struct ieee80211_hw *hw,
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index d1139e4..bbea3bc 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -83,23 +83,6 @@ static inline void drv_bss_info_changed(struct ieee80211_local *local,
trace_drv_bss_info_changed(local, sdata, info, changed);
}

-struct in_ifaddr;
-static inline int drv_configure_arp_filter(struct ieee80211_local *local,
- struct ieee80211_vif *vif,
- struct in_ifaddr *ifa_list)
-{
- int ret = 0;
-
- might_sleep();
-
- if (local->ops->configure_arp_filter)
- ret = local->ops->configure_arp_filter(&local->hw, vif,
- ifa_list);
-
- trace_drv_configure_arp_filter(local, vif_to_sdata(vif), ifa_list, ret);
- return ret;
-}
-
static inline u64 drv_prepare_multicast(struct ieee80211_local *local,
struct netdev_hw_addr_list *mc_list)
{
diff --git a/net/mac80211/driver-trace.h b/net/mac80211/driver-trace.h
index 6b90630..1e293df 100644
--- a/net/mac80211/driver-trace.h
+++ b/net/mac80211/driver-trace.h
@@ -219,31 +219,6 @@ TRACE_EVENT(drv_bss_info_changed,
)
);

-TRACE_EVENT(drv_configure_arp_filter,
- TP_PROTO(struct ieee80211_local *local,
- struct ieee80211_sub_if_data *sdata,
- struct in_ifaddr *ifa_list, int ret),
-
- TP_ARGS(local, sdata, ifa_list, ret),
-
- TP_STRUCT__entry(
- LOCAL_ENTRY
- VIF_ENTRY
- __field(int, ret)
- ),
-
- TP_fast_assign(
- LOCAL_ASSIGN;
- VIF_ASSIGN;
- __entry->ret = ret;
- ),
-
- TP_printk(
- VIF_PR_FMT LOCAL_PR_FMT " ret:%d",
- VIF_PR_ARG, LOCAL_PR_ARG, __entry->ret
- )
-);
-
TRACE_EVENT(drv_prepare_multicast,
TP_PROTO(struct ieee80211_local *local, int mc_count, u64 ret),

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 4d3883e..79e366e 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -517,6 +517,8 @@ struct ieee80211_sub_if_data {

u16 sequence_number;

+ bool arp_filter_state;
+
/*
* AP this belongs to: self in AP mode and
* corresponding AP in VLAN mode, NULL for
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 1afa9ec..1f5476d 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -959,6 +959,9 @@ int ieee80211_if_add(struct ieee80211_local *local, const char *name,
sdata->wdev.wiphy = local->hw.wiphy;
sdata->local = local;
sdata->dev = ndev;
+#ifdef CONFIG_INET
+ sdata->arp_filter_state = true;
+#endif

for (i = 0; i < IEEE80211_FRAGMENT_MAX; i++)
skb_queue_head_init(&sdata->fragments[i].skb_list);
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 5706156..f5fe025 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -20,6 +20,7 @@
#include <linux/rtnetlink.h>
#include <linux/bitmap.h>
#include <linux/pm_qos_params.h>
+#include <linux/inetdevice.h>
#include <net/net_namespace.h>
#include <net/cfg80211.h>

@@ -330,23 +331,6 @@ static void ieee80211_recalc_smps_work(struct work_struct *work)
}

#ifdef CONFIG_INET
-int ieee80211_set_arp_filter(struct ieee80211_sub_if_data *sdata)
-{
- struct in_device *idev;
- int ret = 0;
-
- BUG_ON(!sdata);
- ASSERT_RTNL();
-
- idev = sdata->dev->ip_ptr;
- if (!idev)
- return 0;
-
- ret = drv_configure_arp_filter(sdata->local, &sdata->vif,
- idev->ifa_list);
- return ret;
-}
-
static int ieee80211_ifa_changed(struct notifier_block *nb,
unsigned long data, void *arg)
{
@@ -356,8 +340,11 @@ static int ieee80211_ifa_changed(struct notifier_block *nb,
ifa_notifier);
struct net_device *ndev = ifa->ifa_dev->dev;
struct wireless_dev *wdev = ndev->ieee80211_ptr;
+ struct in_device *idev;
struct ieee80211_sub_if_data *sdata;
+ struct ieee80211_bss_conf *bss_conf;
struct ieee80211_if_managed *ifmgd;
+ int c = 0;

/* Make sure it's our interface that got changed */
if (!wdev)
@@ -366,17 +353,44 @@ static int ieee80211_ifa_changed(struct notifier_block *nb,
if (wdev->wiphy != local->hw.wiphy)
return NOTIFY_DONE;

- /* We are concerned about IP addresses only when associated */
sdata = IEEE80211_DEV_TO_SUB_IF(ndev);
+ bss_conf = &sdata->vif.bss_conf;

/* ARP filtering is only supported in managed mode */
if (sdata->vif.type != NL80211_IFTYPE_STATION)
return NOTIFY_DONE;

+ idev = sdata->dev->ip_ptr;
+ if (!idev)
+ return NOTIFY_DONE;
+
ifmgd = &sdata->u.mgd;
mutex_lock(&ifmgd->mtx);
- if (ifmgd->associated)
- ieee80211_set_arp_filter(sdata);
+
+ /* Copy the addresses to the bss_conf list */
+ ifa = idev->ifa_list;
+ while (c < IEEE80211_BSS_ARP_ADDR_LIST_LEN && ifa) {
+ bss_conf->arp_addr_list[c] = ifa->ifa_address;
+ ifa = ifa->ifa_next;
+ c++;
+ }
+
+ /* If not all addresses fit the list, disable filtering */
+ if (ifa) {
+ sdata->arp_filter_state = false;
+ c = 0;
+ } else {
+ sdata->arp_filter_state = true;
+ }
+ bss_conf->arp_addr_cnt = c;
+
+ /* Configure driver only if associated */
+ if (ifmgd->associated) {
+ bss_conf->arp_filter_enabled = sdata->arp_filter_state;
+ ieee80211_bss_info_change_notify(sdata,
+ BSS_CHANGED_ARP_FILTER);
+ }
+
mutex_unlock(&ifmgd->mtx);

return NOTIFY_DONE;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index ac68c41..5740ce1 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -806,11 +806,12 @@ static void ieee80211_set_associated(struct ieee80211_sub_if_data *sdata,
{
struct ieee80211_bss *bss = (void *)cbss->priv;
struct ieee80211_local *local = sdata->local;
+ struct ieee80211_bss_conf *bss_conf = &sdata->vif.bss_conf;

bss_info_changed |= BSS_CHANGED_ASSOC;
/* set timing information */
- sdata->vif.bss_conf.beacon_int = cbss->beacon_interval;
- sdata->vif.bss_conf.timestamp = cbss->tsf;
+ bss_conf->beacon_int = cbss->beacon_interval;
+ bss_conf->timestamp = cbss->tsf;

bss_info_changed |= BSS_CHANGED_BEACON_INT;
bss_info_changed |= ieee80211_handle_bss_capability(sdata,
@@ -835,7 +836,7 @@ static void ieee80211_set_associated(struct ieee80211_sub_if_data *sdata,

ieee80211_led_assoc(local, 1);

- sdata->vif.bss_conf.assoc = 1;
+ bss_conf->assoc = 1;
/*
* For now just always ask the driver to update the basic rateset
* when we have associated, we aren't checking whether it actually
@@ -848,9 +849,15 @@ static void ieee80211_set_associated(struct ieee80211_sub_if_data *sdata,

/* Tell the driver to monitor connection quality (if supported) */
if ((local->hw.flags & IEEE80211_HW_SUPPORTS_CQM_RSSI) &&
- sdata->vif.bss_conf.cqm_rssi_thold)
+ bss_conf->cqm_rssi_thold)
bss_info_changed |= BSS_CHANGED_CQM;

+ /* Enable ARP filtering */
+ if (bss_conf->arp_filter_enabled != sdata->arp_filter_state) {
+ bss_conf->arp_filter_enabled = sdata->arp_filter_state;
+ bss_info_changed |= BSS_CHANGED_ARP_FILTER;
+ }
+
ieee80211_bss_info_change_notify(sdata, bss_info_changed);

mutex_lock(&local->iflist_mtx);
@@ -932,6 +939,12 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,

ieee80211_hw_config(local, config_changed);

+ /* Disable ARP filtering */
+ if (sdata->vif.bss_conf.arp_filter_enabled) {
+ sdata->vif.bss_conf.arp_filter_enabled = false;
+ changed |= BSS_CHANGED_ARP_FILTER;
+ }
+
/* The BSSID (not really interesting) and HT changed */
changed |= BSS_CHANGED_BSSID | BSS_CHANGED_HT;
ieee80211_bss_info_change_notify(sdata, changed);
@@ -2116,19 +2129,9 @@ static enum work_done_result ieee80211_assoc_done(struct ieee80211_work *wk,
cfg80211_send_assoc_timeout(wk->sdata->dev,
wk->filter_ta);
return WORK_DONE_DESTROY;
-#ifdef CONFIG_INET
- } else {
- mutex_unlock(&wk->sdata->u.mgd.mtx);
-
- /*
- * configure ARP filter IP addresses to the driver,
- * intentionally outside the mgd mutex.
- */
- rtnl_lock();
- ieee80211_set_arp_filter(wk->sdata);
- rtnl_unlock();
-#endif
}
+
+ mutex_unlock(&wk->sdata->u.mgd.mtx);
}

cfg80211_send_rx_assoc(wk->sdata->dev, skb->data, skb->len);
--
1.6.3.3



2010-06-10 17:28:32

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCHv4] mac80211: Fix circular locking dependency in ARP filter handling

Hi Juuso,

On Wed, 2010-06-09 at 22:20 -0700, Juuso Oikarinen wrote:
> This is curious. In your source (mine does not seem to fully match what
> you are using) what is the BUG that is getting triggered, main.c:437 ?

Strange ... in my source that is a weird line number also. I think I
know what happened ... after applying your patch my habits took over and
I only rebuilt the driver, not mac80211. I am very sorry for my previous
false report.

I rebuilt everything cleanly. After doing so I have not encountered any
issues.

Thank you very much

Tested-by: Reinette Chatre <[email protected]>

Reinette






2010-06-09 23:27:48

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCHv4] mac80211: Fix circular locking dependency in ARP filter handling

Hi Juuso,

On Wed, 2010-06-09 at 03:43 -0700, Juuso Oikarinen wrote:
> There is a circular locking dependency when configuring the
> hardware ARP filters on association, occurring when flushing the mac80211
> workqueue.

I am currently running with latest wireless-testing (HEAD
5e3283c821c3a7750b1f7bd982b10ebdbab0e155) with your patch "mac80211: Add
netif state checking to ieee80211_ifa_changed" also applied.

This hunk below does not seem to match with what is currently in
wireless-testing.

> @@ -2116,19 +2129,9 @@ static enum work_done_result ieee80211_assoc_done(struct ieee80211_work *wk,
> cfg80211_send_assoc_timeout(wk->sdata->dev,
> wk->filter_ta);
> return WORK_DONE_DESTROY;
> -#ifdef CONFIG_INET
> - } else {
> - mutex_unlock(&wk->sdata->u.mgd.mtx);
> -
> - /*
> - * configure ARP filter IP addresses to the driver,
> - * intentionally outside the mgd mutex.
> - */
> - rtnl_lock();
> - ieee80211_set_arp_filter(wk->sdata);
> - rtnl_unlock();
> -#endif
> }
> +
> + mutex_unlock(&wk->sdata->u.mgd.mtx);
> }
>
> cfg80211_send_rx_assoc(wk->sdata->dev, skb->data, skb->len);
> --

I made some modifications to get the patch to apply to wireless-testing,
not sure what should be done here since the difference seems
significant.

Even so, I tested your new patch and for some reason the BUG returned
(even though I am now running with "mac80211: Add netif state checking
to ieee80211_ifa_changed"

Here is what I get:

[ 390.153170] ------------[ cut here ]------------
[ 390.156016] kernel BUG at /home/wifi/iwlwifi-2.6/net/mac80211/main.c:437!
[ 390.156016] invalid opcode: 0000 [#1] SMP
[ 390.156016] last sysfs file: /sys/devices/virtual/misc/rfkill/dev
[ 390.156016] CPU 0
[ 390.156016] Modules linked in: iwl3945(+) iwlcore mac80211 cfg80211 rfkill binfmt_misc sbs
sbshc ipv6 evdev psmouse serio_raw pcspkr pegasus mii battery container processor video ac b
utton output intel_agp ext3 jbd mbcache sg sd_mod sr_mod cdrom ahci libahci libata ehci_hcd u
hci_hcd scsi_mod usbcore thermal fan thermal_sys
[ 390.156016]
[ 390.156016] Pid: 5171, comm: modprobe Not tainted 2.6.35-rc2-wl-64031-g07c6e79 #211 To be
filled by O.E.M./Montevina platform
[ 390.156016] RIP: 0010:[<ffffffffa026faf2>] [<ffffffffa026faf2>] ieee80211_alloc_hw+0x502/
0x520 [mac80211]
[ 390.156016] RSP: 0018:ffff880025a9fc78 EFLAGS: 00010246
[ 390.156016] RAX: ffff8800241319c0 RBX: ffff8800241302e0 RCX: 0000000000000001
[ 390.156016] RDX: ffffffffa02647a0 RSI: ffffffffa02647a8 RDI: ffffffffa0264a34
[ 390.156016] RBP: ffff880025a9fc98 R08: 0000000000000008 R09: ffffffff8154a5f8
[ 390.156016] R10: 0000000000000001 R11: 0000000000000000 R12: ffff8800241305a0
[ 390.156016] R13: ffffffffa032dfc0 R14: ffff88003b6b6000 R15: ffff880025fa2180
[ 390.156016] FS: 00007f634f2cd6f0(0000) GS:ffff880002000000(0000) knlGS:0000000000000000
[ 390.156016] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 390.156016] CR2: 00007fffc7cf9040 CR3: 000000002552a000 CR4: 00000000000006f0
[ 390.156016] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 390.156016] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 390.156016] Process modprobe (pid: 5171, threadinfo ffff880025a9e000, task ffff880002442df
0)
[ 390.156016] Stack:
[ 390.156016] ffffffffa032e460 ffffffffa032e460 ffff88003b6b6000 ffff88003b6b6000
[ 390.156016] <0> ffff880025a9fcb8 ffffffffa02d553e ffffffffa032e460 ffff88003b6b6088
[ 390.156016] <0> ffff880025a9fd38 ffffffffa03131a7 0000000000000246 ffffffffa032df48
[ 390.156016] Call Trace:
[ 390.156016] [<ffffffffa02d553e>] iwl_alloc_all+0x1e/0x50 [iwlcore]
[ 390.156016] [<ffffffffa03131a7>] iwl3945_pci_probe+0x27/0x1640 [iwl3945]
[ 390.156016] [<ffffffff81348a56>] ? _raw_spin_unlock+0x26/0x30
[ 390.156016] [<ffffffff811d4ca0>] ? pci_match_device+0x20/0xd0
[ 390.156016] [<ffffffff811d4b52>] local_pci_probe+0x12/0x20
[ 390.156016] [<ffffffff811d5c60>] pci_device_probe+0x80/0xa0
[ 390.156016] [<ffffffff8124fe7b>] ? driver_sysfs_add+0x6b/0x90
[ 390.156016] [<ffffffff8124ffb5>] driver_probe_device+0x95/0x1d0
[ 390.156016] [<ffffffff8125018b>] __driver_attach+0x9b/0xb0
[ 390.156016] [<ffffffff812500f0>] ? __driver_attach+0x0/0xb0
[ 390.156016] [<ffffffff8124f65b>] bus_for_each_dev+0x6b/0xa0
[ 390.156016] [<ffffffff8124fe0c>] driver_attach+0x1c/0x20
[ 390.156016] [<ffffffff8124edb5>] bus_add_driver+0x1b5/0x300
[ 390.156016] [<ffffffffa0091000>] ? iwl3945_init+0x0/0x80 [iwl3945]
[ 390.156016] [<ffffffff8125049c>] driver_register+0x7c/0x170
[ 390.156016] [<ffffffffa0091000>] ? iwl3945_init+0x0/0x80 [iwl3945]
[ 390.156016] [<ffffffff811d5f1a>] __pci_register_driver+0x6a/0xf0
[ 390.156016] [<ffffffffa0091000>] ? iwl3945_init+0x0/0x80 [iwl3945]
[ 390.156016] [<ffffffffa009104a>] iwl3945_init+0x4a/0x80 [iwl3945]
[ 390.156016] [<ffffffff810001d8>] do_one_initcall+0x38/0x190
[ 390.156016] [<ffffffff8107ffbe>] sys_init_module+0xbe/0x220
[ 390.156016] [<ffffffff81002c6b>] system_call_fastpath+0x16/0x1b
[ 390.156016] Code: c7 c6 52 fa 2a a0 49 89 84 24 a0 03 00 00 49 89 84 24 a8 03 00 00 e8 2e
64 f5 e0 4c 89 e0 5b 41 5c 41 5d 41 5e c9 c3 0f 0b eb fe <0f> 0b eb fe 0f 0b eb fe 0f 0b eb f
e 0f 0b eb fe 0f 0b eb fe 0f
[ 390.156016] RIP [<ffffffffa026faf2>] ieee80211_alloc_hw+0x502/0x520 [mac80211]
[ 390.156016] RSP <ffff880025a9fc78>
[ 390.515754] ---[ end trace ac1678fc227a51d2 ]---

Reinette



2010-06-10 16:23:22

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCHv4] mac80211: Fix circular locking dependency in ARP filter handling

On Wed, 2010-06-09 at 22:30 -0700, Juuso Oikarinen wrote:
> On Thu, 2010-06-10 at 07:21 +0200, Juuso Oikarinen wrote:
> > On Thu, 2010-06-10 at 01:27 +0200, ext reinette chatre wrote:
> > > Even so, I tested your new patch and for some reason the BUG returned
> > > (even though I am now running with "mac80211: Add netif state checking
> > > to ieee80211_ifa_changed"
> >
> > This is curious. In your source (mine does not seem to fully match what
> > you are using) what is the BUG that is getting triggered, main.c:437 ?
>
> Oh, and did you remember to apply the fix patch for the bug too, as it
> seems not yet be integrated to wireless-testing:
>
> "mac80211: Add netif state checking to ieee80211_ifa_changed"
>

I am running with that patch .... see my comment above ;)

Reinette



2010-06-10 05:21:42

by Juuso Oikarinen

[permalink] [raw]
Subject: Re: [PATCHv4] mac80211: Fix circular locking dependency in ARP filter handling

Hi,

On Thu, 2010-06-10 at 01:27 +0200, ext reinette chatre wrote:
> Hi Juuso,
>
> On Wed, 2010-06-09 at 03:43 -0700, Juuso Oikarinen wrote:
> > There is a circular locking dependency when configuring the
> > hardware ARP filters on association, occurring when flushing the mac80211
> > workqueue.
>
> I am currently running with latest wireless-testing (HEAD
> 5e3283c821c3a7750b1f7bd982b10ebdbab0e155) with your patch "mac80211: Add
> netif state checking to ieee80211_ifa_changed" also applied.
>
> This hunk below does not seem to match with what is currently in
> wireless-testing.
>
> > @@ -2116,19 +2129,9 @@ static enum work_done_result ieee80211_assoc_done(struct ieee80211_work *wk,
> > cfg80211_send_assoc_timeout(wk->sdata->dev,
> > wk->filter_ta);
> > return WORK_DONE_DESTROY;
> > -#ifdef CONFIG_INET
> > - } else {
> > - mutex_unlock(&wk->sdata->u.mgd.mtx);
> > -
> > - /*
> > - * configure ARP filter IP addresses to the driver,
> > - * intentionally outside the mgd mutex.
> > - */
> > - rtnl_lock();
> > - ieee80211_set_arp_filter(wk->sdata);
> > - rtnl_unlock();
> > -#endif
> > }
> > +
> > + mutex_unlock(&wk->sdata->u.mgd.mtx);
> > }
> >
> > cfg80211_send_rx_assoc(wk->sdata->dev, skb->data, skb->len);
> > --
>
> I made some modifications to get the patch to apply to wireless-testing,
> not sure what should be done here since the difference seems
> significant.

There was a small bug in regard of the mutex_unlock after Linville added
those CONFIG_INET thingies. Apparently he had fixed that in parallel,
hence the conflict.

> Even so, I tested your new patch and for some reason the BUG returned
> (even though I am now running with "mac80211: Add netif state checking
> to ieee80211_ifa_changed"

This is curious. In your source (mine does not seem to fully match what
you are using) what is the BUG that is getting triggered, main.c:437 ?

-Juuso

> Here is what I get:
>
> [ 390.153170] ------------[ cut here ]------------
> [ 390.156016] kernel BUG at /home/wifi/iwlwifi-2.6/net/mac80211/main.c:437!
> [ 390.156016] invalid opcode: 0000 [#1] SMP
> [ 390.156016] last sysfs file: /sys/devices/virtual/misc/rfkill/dev
> [ 390.156016] CPU 0
> [ 390.156016] Modules linked in: iwl3945(+) iwlcore mac80211 cfg80211 rfkill binfmt_misc sbs
> sbshc ipv6 evdev psmouse serio_raw pcspkr pegasus mii battery container processor video ac b
> utton output intel_agp ext3 jbd mbcache sg sd_mod sr_mod cdrom ahci libahci libata ehci_hcd u
> hci_hcd scsi_mod usbcore thermal fan thermal_sys
> [ 390.156016]
> [ 390.156016] Pid: 5171, comm: modprobe Not tainted 2.6.35-rc2-wl-64031-g07c6e79 #211 To be
> filled by O.E.M./Montevina platform
> [ 390.156016] RIP: 0010:[<ffffffffa026faf2>] [<ffffffffa026faf2>] ieee80211_alloc_hw+0x502/
> 0x520 [mac80211]
> [ 390.156016] RSP: 0018:ffff880025a9fc78 EFLAGS: 00010246
> [ 390.156016] RAX: ffff8800241319c0 RBX: ffff8800241302e0 RCX: 0000000000000001
> [ 390.156016] RDX: ffffffffa02647a0 RSI: ffffffffa02647a8 RDI: ffffffffa0264a34
> [ 390.156016] RBP: ffff880025a9fc98 R08: 0000000000000008 R09: ffffffff8154a5f8
> [ 390.156016] R10: 0000000000000001 R11: 0000000000000000 R12: ffff8800241305a0
> [ 390.156016] R13: ffffffffa032dfc0 R14: ffff88003b6b6000 R15: ffff880025fa2180
> [ 390.156016] FS: 00007f634f2cd6f0(0000) GS:ffff880002000000(0000) knlGS:0000000000000000
> [ 390.156016] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [ 390.156016] CR2: 00007fffc7cf9040 CR3: 000000002552a000 CR4: 00000000000006f0
> [ 390.156016] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 390.156016] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [ 390.156016] Process modprobe (pid: 5171, threadinfo ffff880025a9e000, task ffff880002442df
> 0)
> [ 390.156016] Stack:
> [ 390.156016] ffffffffa032e460 ffffffffa032e460 ffff88003b6b6000 ffff88003b6b6000
> [ 390.156016] <0> ffff880025a9fcb8 ffffffffa02d553e ffffffffa032e460 ffff88003b6b6088
> [ 390.156016] <0> ffff880025a9fd38 ffffffffa03131a7 0000000000000246 ffffffffa032df48
> [ 390.156016] Call Trace:
> [ 390.156016] [<ffffffffa02d553e>] iwl_alloc_all+0x1e/0x50 [iwlcore]
> [ 390.156016] [<ffffffffa03131a7>] iwl3945_pci_probe+0x27/0x1640 [iwl3945]
> [ 390.156016] [<ffffffff81348a56>] ? _raw_spin_unlock+0x26/0x30
> [ 390.156016] [<ffffffff811d4ca0>] ? pci_match_device+0x20/0xd0
> [ 390.156016] [<ffffffff811d4b52>] local_pci_probe+0x12/0x20
> [ 390.156016] [<ffffffff811d5c60>] pci_device_probe+0x80/0xa0
> [ 390.156016] [<ffffffff8124fe7b>] ? driver_sysfs_add+0x6b/0x90
> [ 390.156016] [<ffffffff8124ffb5>] driver_probe_device+0x95/0x1d0
> [ 390.156016] [<ffffffff8125018b>] __driver_attach+0x9b/0xb0
> [ 390.156016] [<ffffffff812500f0>] ? __driver_attach+0x0/0xb0
> [ 390.156016] [<ffffffff8124f65b>] bus_for_each_dev+0x6b/0xa0
> [ 390.156016] [<ffffffff8124fe0c>] driver_attach+0x1c/0x20
> [ 390.156016] [<ffffffff8124edb5>] bus_add_driver+0x1b5/0x300
> [ 390.156016] [<ffffffffa0091000>] ? iwl3945_init+0x0/0x80 [iwl3945]
> [ 390.156016] [<ffffffff8125049c>] driver_register+0x7c/0x170
> [ 390.156016] [<ffffffffa0091000>] ? iwl3945_init+0x0/0x80 [iwl3945]
> [ 390.156016] [<ffffffff811d5f1a>] __pci_register_driver+0x6a/0xf0
> [ 390.156016] [<ffffffffa0091000>] ? iwl3945_init+0x0/0x80 [iwl3945]
> [ 390.156016] [<ffffffffa009104a>] iwl3945_init+0x4a/0x80 [iwl3945]
> [ 390.156016] [<ffffffff810001d8>] do_one_initcall+0x38/0x190
> [ 390.156016] [<ffffffff8107ffbe>] sys_init_module+0xbe/0x220
> [ 390.156016] [<ffffffff81002c6b>] system_call_fastpath+0x16/0x1b
> [ 390.156016] Code: c7 c6 52 fa 2a a0 49 89 84 24 a0 03 00 00 49 89 84 24 a8 03 00 00 e8 2e
> 64 f5 e0 4c 89 e0 5b 41 5c 41 5d 41 5e c9 c3 0f 0b eb fe <0f> 0b eb fe 0f 0b eb fe 0f 0b eb f
> e 0f 0b eb fe 0f 0b eb fe 0f
> [ 390.156016] RIP [<ffffffffa026faf2>] ieee80211_alloc_hw+0x502/0x520 [mac80211]
> [ 390.156016] RSP <ffff880025a9fc78>
> [ 390.515754] ---[ end trace ac1678fc227a51d2 ]---
>
> Reinette
>
>



2010-06-10 05:31:15

by Juuso Oikarinen

[permalink] [raw]
Subject: Re: [PATCHv4] mac80211: Fix circular locking dependency in ARP filter handling

On Thu, 2010-06-10 at 07:21 +0200, Juuso Oikarinen wrote:
> Hi,
>
> On Thu, 2010-06-10 at 01:27 +0200, ext reinette chatre wrote:
> > Hi Juuso,
> >
> > Even so, I tested your new patch and for some reason the BUG returned
> > (even though I am now running with "mac80211: Add netif state checking
> > to ieee80211_ifa_changed"
>
> This is curious. In your source (mine does not seem to fully match what
> you are using) what is the BUG that is getting triggered, main.c:437 ?

Oh, and did you remember to apply the fix patch for the bug too, as it
seems not yet be integrated to wireless-testing:

"mac80211: Add netif state checking to ieee80211_ifa_changed"

-Juuso

> -Juuso
>
> > Here is what I get:
> >
> > [ 390.153170] ------------[ cut here ]------------
> > [ 390.156016] kernel BUG at /home/wifi/iwlwifi-2.6/net/mac80211/main.c:437!
> > [ 390.156016] invalid opcode: 0000 [#1] SMP
> > [ 390.156016] last sysfs file: /sys/devices/virtual/misc/rfkill/dev
> > [ 390.156016] CPU 0
> > [ 390.156016] Modules linked in: iwl3945(+) iwlcore mac80211 cfg80211 rfkill binfmt_misc sbs