Message ID | 1498338507-3432-1-git-send-email-arend.vanspriel@broadcom.com (mailing list archive) |
---|---|
State | Accepted |
Commit | dca2307ed6250e7be6993e165570fa4097903a58 |
Delegated to: | Kalle Valo |
Headers | show |
Arend Van Spriel <arend.vanspriel@broadcom.com> wrote: > The function brcmf_net_attach() can only fail when register_netdevice() > fails. When this happens register_netdevice() calls priv_destructor, ie. > brcmf_cfg80211_free_netdev() freeing the vif instance. Also upon this > failure brcmf_net_attach() calls free_netdev(). However, callers are also > doing cleanup resulting in double free. In some places they need netdev > private space as it holds parameters to communicate with the device. So > we want to do the cleanup only in callers of brcmf_net_attach() by making > the following changes: > > - set priv_destructor after register_netdevice() succeeds. > - remove call to free_netdev() in brcmf_net_attach(). > - call free_netdev() in brcmf_net_detach() for unregistered netdev. > - add free_netdev() if brcmf_net_attach() fails for a created interface. > > Fixes: cf124db566e6 ("net: Fix inconsistent teardown and release of private netdev state.") > Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com> > Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com> > Reviewed-by: Franky Lin <franky.lin@broadcom.com> > Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com> Patch applied to wireless-drivers-next.git, thanks. dca2307ed625 brcmfmac: fix double free upon register_netdevice() failure
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c index 2443c71..63e7683 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c @@ -625,6 +625,7 @@ struct wireless_dev *brcmf_ap_add_vif(struct wiphy *wiphy, const char *name, err = brcmf_net_attach(ifp, true); if (err) { brcmf_err("Registering netdevice failed\n"); + free_netdev(ifp->ndev); goto fail; } diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c index c60b897..b4c8643 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c @@ -485,13 +485,13 @@ int brcmf_net_attach(struct brcmf_if *ifp, bool rtnl_locked) goto fail; } + ndev->priv_destructor = brcmf_cfg80211_free_netdev; brcmf_dbg(INFO, "%s: Broadcom Dongle Host Driver\n", ndev->name); return 0; fail: drvr->iflist[ifp->bsscfgidx] = NULL; ndev->netdev_ops = NULL; - free_netdev(ndev); return -EBADE; } @@ -504,6 +504,7 @@ static void brcmf_net_detach(struct net_device *ndev, bool rtnl_locked) unregister_netdev(ndev); } else { brcmf_cfg80211_free_netdev(ndev); + free_netdev(ndev); } } @@ -580,7 +581,6 @@ static int brcmf_net_p2p_attach(struct brcmf_if *ifp) fail: ifp->drvr->iflist[ifp->bsscfgidx] = NULL; ndev->netdev_ops = NULL; - free_netdev(ndev); return -EBADE; } @@ -626,7 +626,6 @@ struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 bsscfgidx, s32 ifidx, return ERR_PTR(-ENOMEM); ndev->needs_free_netdev = true; - ndev->priv_destructor = brcmf_cfg80211_free_netdev; ifp = netdev_priv(ndev); ifp->ndev = ndev; /* store mapping ifidx to bsscfgidx */ diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c index aa299c4..2ce675a 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c @@ -2208,6 +2208,7 @@ struct wireless_dev *brcmf_p2p_add_vif(struct wiphy *wiphy, const char *name, err = brcmf_net_attach(ifp, true); if (err) { brcmf_err("Registering netdevice failed\n"); + free_netdev(ifp->ndev); goto fail; }