[PATCH 4.7 FIX] brcmfmac: fix lockup when removing P2P interface after event timeout

[PATCH 4.7 FIX] brcmfmac: fix lockup when removing P2P interface after event timeout

[Rafał Miłecki]

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>
---
 .../broadcom/brcm80211/brcmfmac/cfg80211.c         |  2 +-
 .../wireless/broadcom/brcm80211/brcmfmac/core.c    | 29 +++++++++++++---------
 .../wireless/broadcom/brcm80211/brcmfmac/core.h    |  2 +-
 .../wireless/broadcom/brcm80211/brcmfmac/fweh.c    |  2 +-
 .../net/wireless/broadcom/brcm80211/brcmfmac/p2p.c |  4 +--
 5 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 62f475e..ef09382 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5364,7 +5364,7 @@ brcmf_notify_connect_status_ap(struct brcmf_cfg80211_info *cfg,
 		brcmf_dbg(CONN, "AP mode link down\n");
 		complete(&cfg->vif_disabled);
 		if (ifp->vif->mbss)
-			brcmf_remove_interface(ifp);
+			brcmf_remove_interface(ifp, false);
 		return 0;
 	}
 
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index b590499..7f8ba48c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -548,12 +548,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)
@@ -651,7 +655,7 @@ struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 bsscfgidx, s32 ifidx,
 			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",
@@ -699,7 +703,8 @@ struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 bsscfgidx, s32 ifidx,
 	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;
 
@@ -729,7 +734,7 @@ static void brcmf_del_if(struct brcmf_pub *drvr, s32 bsscfgidx)
 			cancel_work_sync(&ifp->multicast_work);
 			cancel_work_sync(&ifp->ndoffload_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
@@ -743,14 +748,14 @@ static void brcmf_del_if(struct brcmf_pub *drvr, s32 bsscfgidx)
 	}
 }
 
-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);
 }
 
 int brcmf_get_next_free_bsscfgidx(struct brcmf_pub *drvr)
@@ -1081,9 +1086,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 (drvr->settings->ignore_probe_fail)
@@ -1152,7 +1157,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);
 
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
index 647d3cc..5c5a392 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
@@ -216,7 +216,7 @@ struct brcmf_if *brcmf_get_ifp(struct brcmf_pub *drvr, int ifidx);
 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);
 int brcmf_get_next_free_bsscfgidx(struct brcmf_pub *drvr);
 void brcmf_txflowblock_if(struct brcmf_if *ifp,
 			  enum brcmf_netif_stop_reason reason, bool state);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
index b390561..9da7a4c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
@@ -183,7 +183,7 @@ static void brcmf_fweh_handle_if_event(struct brcmf_pub *drvr,
 	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);
 }
 
 /**
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
index a70cda6..c4dc6be 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -2290,7 +2290,7 @@ int brcmf_p2p_del_vif(struct wiphy *wiphy, struct wireless_dev *wdev)
 			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)
@@ -2396,7 +2396,7 @@ void brcmf_p2p_detach(struct brcmf_p2p_info *p2p)
 	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));
-- 
1.8.4.5

Re: [PATCH 4.7 FIX] brcmfmac: fix lockup when removing P2P interface after event timeout

[Kalle Valo]

Rafał Miłecki <zajec5@gmail.com> writes:

> 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>

Arend, are you ok to push this to 4.7?

-- 
Kalle Valo

Re: [PATCH 4.7 FIX] brcmfmac: fix lockup when removing P2P interface after event timeout

[Kalle Valo]

Rafał Miłecki <zajec5@gmail.com> writes:

> 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>

Failed to apply, please rebase:

Applying: brcmfmac: fix lockup when removing P2P interface after event timeout
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
CONFLICT (content): Merge conflict in drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
Failed to merge in the changes.
Patch failed at 0001 brcmfmac: fix lockup when removing P2P interface after event timeout

-- 
Kalle Valo

Re: [PATCH 4.7 FIX] brcmfmac: fix lockup when removing P2P interface after event timeout

[Rafał Miłecki]

On 16 June 2016 at 17:10, Kalle Valo <kvalo@codeaurora.org> wrote:
> Rafał Miłecki <zajec5@gmail.com> writes:
>
>> 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>
>
> Failed to apply, please rebase:
>
> Applying: brcmfmac: fix lockup when removing P2P interface after event timeout
> Using index info to reconstruct a base tree...
> Falling back to patching base and 3-way merge...
> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
> CONFLICT (content): Merge conflict in drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> Failed to merge in the changes.
> Patch failed at 0001 brcmfmac: fix lockup when removing P2P interface after event timeout

What tree did you try it on?

I just went into a dir where I have cloned:
git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git

My HEAD commit is:
034fdd4 Merge ath-current from ath.git

And I can apply this patch cleanly doing:
curl https://patchwork.kernel.org/patch/9138925/mbox/ | git am

-- 
Rafał

Re: [PATCH 4.7 FIX] brcmfmac: fix lockup when removing P2P interface after event timeout

[Kalle Valo]

Rafał Miłecki <zajec5@gmail.com> writes:

> On 16 June 2016 at 17:10, Kalle Valo <kvalo@codeaurora.org> wrote:
>> Rafał Miłecki <zajec5@gmail.com> writes:
>>
>>> 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>
>>
>> Failed to apply, please rebase:
>>
>> Applying: brcmfmac: fix lockup when removing P2P interface after event timeout
>> Using index info to reconstruct a base tree...
>> Falling back to patching base and 3-way merge...
>> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
>> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
>> CONFLICT (content): Merge conflict in drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
>> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> Failed to merge in the changes.
>> Patch failed at 0001 brcmfmac: fix lockup when removing P2P interface after event timeout
>
> What tree did you try it on?
>
> I just went into a dir where I have cloned:
> git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git
>
> My HEAD commit is:
> 034fdd4 Merge ath-current from ath.git
>
> And I can apply this patch cleanly doing:
> curl https://patchwork.kernel.org/patch/9138925/mbox/ | git am

I was trying to apply this to wireless-drivers-next. I didn't get a
confirmation from Arend and I didn't consider the fix important enough
for 4.7. But of course I can reconsider if needed.

-- 
Kalle Valo

Re: [PATCH 4.7 FIX] brcmfmac: fix lockup when removing P2P interface after event timeout

[Rafał Miłecki]

On 17 June 2016 at 07:13, Kalle Valo <kvalo@codeaurora.org> wrote:
> Rafał Miłecki <zajec5@gmail.com> writes:
>
>> On 16 June 2016 at 17:10, Kalle Valo <kvalo@codeaurora.org> wrote:
>>> Rafał Miłecki <zajec5@gmail.com> writes:
>>>
>>>> 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>
>>>
>>> Failed to apply, please rebase:
>>>
>>> Applying: brcmfmac: fix lockup when removing P2P interface after event timeout
>>> Using index info to reconstruct a base tree...
>>> Falling back to patching base and 3-way merge...
>>> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
>>> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
>>> CONFLICT (content): Merge conflict in drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
>>> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>>> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> Failed to merge in the changes.
>>> Patch failed at 0001 brcmfmac: fix lockup when removing P2P interface after event timeout
>>
>> What tree did you try it on?
>>
>> I just went into a dir where I have cloned:
>> git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git
>>
>> My HEAD commit is:
>> 034fdd4 Merge ath-current from ath.git
>>
>> And I can apply this patch cleanly doing:
>> curl https://patchwork.kernel.org/patch/9138925/mbox/ | git am
>
> I was trying to apply this to wireless-drivers-next. I didn't get a
> confirmation from Arend and I didn't consider the fix important enough
> for 4.7. But of course I can reconsider if needed.

I think I agree it won't hurt to get it into -next. Noone earlier
reported this bug and it seems to be there for a long time. Also
applying it to -next will allow avoiding merge conflicts and immediate
development work on -next.

I'll resend this patch rebased on -next soon.

-- 
Rafał

Re: [PATCH 4.7 FIX] brcmfmac: fix lockup when removing P2P interface after event timeout

[Kalle Valo]

Rafał Miłecki <zajec5@gmail.com> writes:

>>>> Failed to apply, please rebase:
>>>>
>>>> Applying: brcmfmac: fix lockup when removing P2P interface after event timeout
>>>> Using index info to reconstruct a base tree...
>>>> Falling back to patching base and 3-way merge...
>>>> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
>>>> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
>>>> CONFLICT (content): Merge conflict in drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
>>>> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>>>> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>>> Failed to merge in the changes.
>>>> Patch failed at 0001 brcmfmac: fix lockup when removing P2P interface after event timeout
>>>
>>> What tree did you try it on?
>>>
>>> I just went into a dir where I have cloned:
>>> git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git
>>>
>>> My HEAD commit is:
>>> 034fdd4 Merge ath-current from ath.git
>>>
>>> And I can apply this patch cleanly doing:
>>> curl https://patchwork.kernel.org/patch/9138925/mbox/ | git am
>>
>> I was trying to apply this to wireless-drivers-next. I didn't get a
>> confirmation from Arend and I didn't consider the fix important enough
>> for 4.7. But of course I can reconsider if needed.
>
> I think I agree it won't hurt to get it into -next. Noone earlier
> reported this bug and it seems to be there for a long time. Also
> applying it to -next will allow avoiding merge conflicts and immediate
> development work on -next.

Exactly, there is a cost when taking patches to wireless-drivers.git and
that's why I try to keep the bar high. Regressions and user reported
bugs take priority, other fixes are handled case by case.

> I'll resend this patch rebased on -next soon.

Thanks.

-- 
Kalle Valo

[PATCH REBASED] brcmfmac: fix lockup when removing P2P interface after event timeout

[Rafał Miłecki]

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>
---
REBASED on top of wireless-drivers-next. This bug isn't very common and
changes may conflict with other patches, so it makes more sense to pick
it for -next.
---
 .../wireless/broadcom/brcm80211/brcmfmac/core.c    | 29 +++++++++++++---------
 .../wireless/broadcom/brcm80211/brcmfmac/core.h    |  2 +-
 .../wireless/broadcom/brcm80211/brcmfmac/fweh.c    |  2 +-
 .../net/wireless/broadcom/brcm80211/brcmfmac/p2p.c |  4 +--
 4 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index faf4e46..7b38c2b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -548,12 +548,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)
@@ -651,7 +655,7 @@ struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 bsscfgidx, s32 ifidx,
 			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",
@@ -699,7 +703,8 @@ struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 bsscfgidx, s32 ifidx,
 	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;
 
@@ -729,7 +734,7 @@ static void brcmf_del_if(struct brcmf_pub *drvr, s32 bsscfgidx)
 			cancel_work_sync(&ifp->multicast_work);
 			cancel_work_sync(&ifp->ndoffload_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
@@ -743,14 +748,14 @@ static void brcmf_del_if(struct brcmf_pub *drvr, s32 bsscfgidx)
 	}
 }
 
-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
@@ -1057,9 +1062,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 (drvr->settings->ignore_probe_fail)
@@ -1128,7 +1133,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);
 
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
index 2a075c5..a0a6f7f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
@@ -216,7 +216,7 @@ struct brcmf_if *brcmf_get_ifp(struct brcmf_pub *drvr, int ifidx);
 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);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
index b390561..9da7a4c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
@@ -183,7 +183,7 @@ static void brcmf_fweh_handle_if_event(struct brcmf_pub *drvr,
 	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);
 }
 
 /**
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
index f38a821..426ff05 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -2287,7 +2287,7 @@ int brcmf_p2p_del_vif(struct wiphy *wiphy, struct wireless_dev *wdev)
 			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)
@@ -2393,7 +2393,7 @@ void brcmf_p2p_detach(struct brcmf_p2p_info *p2p)
 	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));
-- 
1.8.4.5

[PATCH] brcmfmac: use const char * for interface name in brcmf_add_if

[Rafał Miłecki]

This function can work just fine with const pointer, it only calls
alloc_netdev which take const as well. Moreover it makes this function
more flexible as some cfg80211 callback may provide const char * as
well, e.g. add_virtual_intf. This will be needed for more advanced
interface management.

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 2 +-
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 7b38c2b..8d16f02 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -638,7 +638,7 @@ fail:
 }
 
 struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 bsscfgidx, s32 ifidx,
-			      bool is_p2pdev, char *name, u8 *mac_addr)
+			      bool is_p2pdev, const char *name, u8 *mac_addr)
 {
 	struct brcmf_if *ifp;
 	struct net_device *ndev;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
index a0a6f7f..8fa34ca 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
@@ -215,7 +215,7 @@ char *brcmf_ifname(struct brcmf_if *ifp);
 struct brcmf_if *brcmf_get_ifp(struct brcmf_pub *drvr, int ifidx);
 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);
+			      bool is_p2pdev, const char *name, u8 *mac_addr);
 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);
-- 
1.8.4.5

Re: [PATCH] brcmfmac: use const char * for interface name in brcmf_add_if

[Joe Perches]

On Fri, 2016-06-17 at 12:48 +0200, Rafał Miłecki wrote:
> This function can work just fine with const pointer, it only calls
> alloc_netdev which take const as well. Moreover it makes this function
> more flexible as some cfg80211 callback may provide const char * as
> well, e.g. add_virtual_intf. This will be needed for more advanced
> interface management.
[]
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
[]
> @@ -638,7 +638,7 @@ fail:
>  }
>  
>  struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 bsscfgidx, s32 ifidx,
> -			      bool is_p2pdev, char *name, u8 *mac_addr)
> +			      bool is_p2pdev, const char *name, u8 *mac_addr)

Looks like mac_addr should be const too

>  {
>  	struct brcmf_if *ifp;
>  	struct net_device *ndev;
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
[]
> @@ -215,7 +215,7 @@ char *brcmf_ifname(struct brcmf_if *ifp);
>  struct brcmf_if *brcmf_get_ifp(struct brcmf_pub *drvr, int ifidx);
>  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);
> +			      bool is_p2pdev, const char *name, u8 *mac_addr);
>  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);

Re: [REBASED] brcmfmac: fix lockup when removing P2P interface after event timeout

[Kalle Valo]

Rafał Miłecki wrote:
> 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>

Thanks, 1 patch applied to wireless-drivers-next.git:

b50ddfa8530e brcmfmac: fix lockup when removing P2P interface after event timeout

-- 
Sent by pwcli
https://patchwork.kernel.org/patch/9183337/

Re: brcmfmac: use const char * for interface name in brcmf_add_if

[Kalle Valo]

Rafał Miłecki wrote:
> This function can work just fine with const pointer, it only calls
> alloc_netdev which take const as well. Moreover it makes this function
> more flexible as some cfg80211 callback may provide const char * as
> well, e.g. add_virtual_intf. This will be needed for more advanced
> interface management.
> 
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>

Thanks, 1 patch applied to wireless-drivers-next.git:

54264e7ea09a brcmfmac: use const char * for interface name in brcmf_add_if

-- 
Sent by pwcli
https://patchwork.kernel.org/patch/9183453/