Skip to content

Commit

Permalink
mac80211: brcmfmac: fix lockup related to P2P interface
Browse files Browse the repository at this point in the history
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
  • Loading branch information
rmilecki authored and wigyori committed Sep 29, 2016
1 parent 29fcc94 commit a047169
Show file tree
Hide file tree
Showing 4 changed files with 300 additions and 1 deletion.
@@ -0,0 +1,37 @@
From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= <zajec5@gmail.com>
Date: Tue, 7 Jun 2016 08:20:21 +0200
Subject: [PATCH] brcmfmac: include required headers in cfg80211.h
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Without this including cfg80211.h in a wrong order could result in:

drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h:122:24: error: array type has incomplete element type
struct brcmf_wsec_key key[BRCMF_MAX_DEFAULT_KEYS];
^
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h:291:24: error: field ‘p2p’ has incomplete type
struct brcmf_p2p_info p2p;
^
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h:297:27: error: field ‘pmk_list’ has incomplete type
struct brcmf_pmk_list_le pmk_list;
^
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h:317:28: error: field ‘assoclist’ has incomplete type
struct brcmf_assoclist_le assoclist;

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
---

--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
@@ -20,6 +20,9 @@
/* for brcmu_d11inf */
#include <brcmu_d11.h>

+#include "fwil_types.h"
+#include "p2p.h"
+
#define WL_NUM_SCAN_MAX 10
#define WL_TLV_INFO_MAX 1024
#define WL_BSS_INFO_MAX 2048
@@ -0,0 +1,108 @@
From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= <zajec5@gmail.com>
Date: Tue, 7 Jun 2016 21:10:18 +0200
Subject: [PATCH] brcmfmac: slightly simplify building interface combinations
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This change reorders some operations in brcmf_setup_ifmodes in hope to
make it simpler:
1) It allocates arrays right before filling them. This way it's easier
to follow requested array length as it's immediately followed by
code filling it. It's easier to check e.g. why we need 4 entries for
P2P. Other than that it deduplicates some checks (e.g. for P2P).
2) It reorders code to first prepare limits and then define a new combo.
Previously this was mixed (e.g. we were setting num of channels
before preparing limits).
3) It modifies mbss code to use i variable just like other combos do.

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
---

--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -6108,29 +6108,15 @@ static int brcmf_setup_ifmodes(struct wi
if (!combo)
goto err;

- c0_limits = kcalloc(p2p ? 3 : 2, sizeof(*c0_limits), GFP_KERNEL);
- if (!c0_limits)
- goto err;
-
- if (p2p) {
- p2p_limits = kcalloc(4, sizeof(*p2p_limits), GFP_KERNEL);
- if (!p2p_limits)
- goto err;
- }
-
- if (mbss) {
- mbss_limits = kcalloc(1, sizeof(*mbss_limits), GFP_KERNEL);
- if (!mbss_limits)
- goto err;
- }
-
wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
BIT(NL80211_IFTYPE_ADHOC) |
BIT(NL80211_IFTYPE_AP);

c = 0;
i = 0;
- combo[c].num_different_channels = 1;
+ c0_limits = kcalloc(p2p ? 3 : 2, sizeof(*c0_limits), GFP_KERNEL);
+ if (!c0_limits)
+ goto err;
c0_limits[i].max = 1;
c0_limits[i++].types = BIT(NL80211_IFTYPE_STATION);
if (p2p) {
@@ -6148,6 +6134,7 @@ static int brcmf_setup_ifmodes(struct wi
c0_limits[i].max = 1;
c0_limits[i++].types = BIT(NL80211_IFTYPE_AP);
}
+ combo[c].num_different_channels = 1;
combo[c].max_interfaces = i;
combo[c].n_limits = i;
combo[c].limits = c0_limits;
@@ -6155,7 +6142,9 @@ static int brcmf_setup_ifmodes(struct wi
if (p2p) {
c++;
i = 0;
- combo[c].num_different_channels = 1;
+ p2p_limits = kcalloc(4, sizeof(*p2p_limits), GFP_KERNEL);
+ if (!p2p_limits)
+ goto err;
p2p_limits[i].max = 1;
p2p_limits[i++].types = BIT(NL80211_IFTYPE_STATION);
p2p_limits[i].max = 1;
@@ -6164,6 +6153,7 @@ static int brcmf_setup_ifmodes(struct wi
p2p_limits[i++].types = BIT(NL80211_IFTYPE_P2P_CLIENT);
p2p_limits[i].max = 1;
p2p_limits[i++].types = BIT(NL80211_IFTYPE_P2P_DEVICE);
+ combo[c].num_different_channels = 1;
combo[c].max_interfaces = i;
combo[c].n_limits = i;
combo[c].limits = p2p_limits;
@@ -6171,14 +6161,19 @@ static int brcmf_setup_ifmodes(struct wi

if (mbss) {
c++;
+ i = 0;
+ mbss_limits = kcalloc(1, sizeof(*mbss_limits), GFP_KERNEL);
+ if (!mbss_limits)
+ goto err;
+ mbss_limits[i].max = 4;
+ mbss_limits[i++].types = BIT(NL80211_IFTYPE_AP);
combo[c].beacon_int_infra_match = true;
combo[c].num_different_channels = 1;
- mbss_limits[0].max = 4;
- mbss_limits[0].types = BIT(NL80211_IFTYPE_AP);
combo[c].max_interfaces = 4;
- combo[c].n_limits = 1;
+ combo[c].n_limits = i;
combo[c].limits = mbss_limits;
}
+
wiphy->n_iface_combinations = n_combos;
wiphy->iface_combinations = combo;
return 0;
@@ -0,0 +1,154 @@
From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= <zajec5@gmail.com>
Date: Fri, 17 Jun 2016 12:29:21 +0200
Subject: [PATCH] brcmfmac: fix lockup when removing P2P interface after event
timeout
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Removing P2P interface is handled by sending a proper request to the
firmware. On success firmware triggers an event and driver's handler
removes a matching interface.

However on event timeout we remove interface directly from the cfg80211
callback. Current code doesn't handle this case correctly as it always
assumes rtnl to be unlocked.

Fix it by adding an extra rtnl_locked parameter to functions and calling
unregister_netdevice when needed.

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
---

--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -705,12 +705,16 @@ fail:
return -EBADE;
}

-static void brcmf_net_detach(struct net_device *ndev)
+static void brcmf_net_detach(struct net_device *ndev, bool rtnl_locked)
{
- if (ndev->reg_state == NETREG_REGISTERED)
- unregister_netdev(ndev);
- else
+ if (ndev->reg_state == NETREG_REGISTERED) {
+ if (rtnl_locked)
+ unregister_netdevice(ndev);
+ else
+ unregister_netdev(ndev);
+ } else {
brcmf_cfg80211_free_netdev(ndev);
+ }
}

void brcmf_net_setcarrier(struct brcmf_if *ifp, bool on)
@@ -808,7 +812,7 @@ struct brcmf_if *brcmf_add_if(struct brc
brcmf_err("ERROR: netdev:%s already exists\n",
ifp->ndev->name);
netif_stop_queue(ifp->ndev);
- brcmf_net_detach(ifp->ndev);
+ brcmf_net_detach(ifp->ndev, false);
drvr->iflist[bsscfgidx] = NULL;
} else {
brcmf_dbg(INFO, "netdev:%s ignore IF event\n",
@@ -856,7 +860,8 @@ struct brcmf_if *brcmf_add_if(struct brc
return ifp;
}

-static void brcmf_del_if(struct brcmf_pub *drvr, s32 bsscfgidx)
+static void brcmf_del_if(struct brcmf_pub *drvr, s32 bsscfgidx,
+ bool rtnl_locked)
{
struct brcmf_if *ifp;

@@ -885,7 +890,7 @@ static void brcmf_del_if(struct brcmf_pu
cancel_work_sync(&ifp->setmacaddr_work);
cancel_work_sync(&ifp->multicast_work);
}
- brcmf_net_detach(ifp->ndev);
+ brcmf_net_detach(ifp->ndev, rtnl_locked);
} else {
/* Only p2p device interfaces which get dynamically created
* end up here. In this case the p2p module should be informed
@@ -899,14 +904,14 @@ static void brcmf_del_if(struct brcmf_pu
}
}

-void brcmf_remove_interface(struct brcmf_if *ifp)
+void brcmf_remove_interface(struct brcmf_if *ifp, bool rtnl_locked)
{
if (!ifp || WARN_ON(ifp->drvr->iflist[ifp->bsscfgidx] != ifp))
return;
brcmf_dbg(TRACE, "Enter, bsscfgidx=%d, ifidx=%d\n", ifp->bsscfgidx,
ifp->ifidx);
brcmf_fws_del_interface(ifp);
- brcmf_del_if(ifp->drvr, ifp->bsscfgidx);
+ brcmf_del_if(ifp->drvr, ifp->bsscfgidx, rtnl_locked);
}

#ifdef CONFIG_INET
@@ -1154,9 +1159,9 @@ fail:
brcmf_fws_deinit(drvr);
}
if (ifp)
- brcmf_net_detach(ifp->ndev);
+ brcmf_net_detach(ifp->ndev, false);
if (p2p_ifp)
- brcmf_net_detach(p2p_ifp->ndev);
+ brcmf_net_detach(p2p_ifp->ndev, false);
drvr->iflist[0] = NULL;
drvr->iflist[1] = NULL;
if (brcmf_ignoring_probe_fail(drvr))
@@ -1222,7 +1227,7 @@ void brcmf_detach(struct device *dev)

/* make sure primary interface removed last */
for (i = BRCMF_MAX_IFS-1; i > -1; i--)
- brcmf_remove_interface(drvr->iflist[i]);
+ brcmf_remove_interface(drvr->iflist[i], false);

brcmf_cfg80211_detach(drvr->config);

--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
@@ -213,7 +213,7 @@ struct brcmf_if *brcmf_get_ifp(struct br
int brcmf_net_attach(struct brcmf_if *ifp, bool rtnl_locked);
struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 bsscfgidx, s32 ifidx,
bool is_p2pdev, char *name, u8 *mac_addr);
-void brcmf_remove_interface(struct brcmf_if *ifp);
+void brcmf_remove_interface(struct brcmf_if *ifp, bool rtnl_locked);
void brcmf_txflowblock_if(struct brcmf_if *ifp,
enum brcmf_netif_stop_reason reason, bool state);
void brcmf_txfinalize(struct brcmf_if *ifp, struct sk_buff *txp, bool success);
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
@@ -226,7 +226,7 @@ static void brcmf_fweh_handle_if_event(s
err = brcmf_fweh_call_event_handler(ifp, emsg->event_code, emsg, data);

if (ifp && ifevent->action == BRCMF_E_IF_DEL)
- brcmf_remove_interface(ifp);
+ brcmf_remove_interface(ifp, false);
}

/**
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -2280,7 +2280,7 @@ int brcmf_p2p_del_vif(struct wiphy *wiph
err = 0;
}
if (err)
- brcmf_remove_interface(vif->ifp);
+ brcmf_remove_interface(vif->ifp, true);

brcmf_cfg80211_arm_vif_event(cfg, NULL);
if (vif->wdev.iftype != NL80211_IFTYPE_P2P_DEVICE)
@@ -2386,7 +2386,7 @@ void brcmf_p2p_detach(struct brcmf_p2p_i
if (vif != NULL) {
brcmf_p2p_cancel_remain_on_channel(vif->ifp);
brcmf_p2p_deinit_discovery(p2p);
- brcmf_remove_interface(vif->ifp);
+ brcmf_remove_interface(vif->ifp, false);
}
/* just set it all to zero */
memset(p2p, 0, sizeof(*p2p));
Expand Up @@ -13,7 +13,7 @@ Signed-off-by: Rafał Miłecki <zajec5@gmail.com>

--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -1308,6 +1308,7 @@ static int __init brcmfmac_module_init(v
@@ -1313,6 +1313,7 @@ static int __init brcmfmac_module_init(v
#endif
if (!schedule_work(&brcmf_driver_work))
return -EBUSY;
Expand Down

0 comments on commit a047169

Please sign in to comment.