| Message ID | 1497258385-5116-1-git-send-email-arend.vanspriel@broadcom.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, 2017-06-12 at 10:06 +0100, Arend van Spriel wrote: > This patch deals with changes made in struct net_device by commit > cf124db566e6 ("net: Fix inconsistent teardown and release of private > netdev state."). This only looks for instances that need > free_netdev() call, ie. struct net_device::needs_free_netdev == true. I think we may need to worry about more kernel versions than just 4.12 here, since this stuff is getting backported, and then it would cause double-frees. I'm also considering simply removing the priv_destructor entirely from mac80211, but I guess this would still apply to other drivers. johannes -- To unsubscribe from this list: send the line "unsubscribe backports" in
On 6/12/2017 11:11 AM, Johannes Berg wrote: > On Mon, 2017-06-12 at 10:06 +0100, Arend van Spriel wrote: >> This patch deals with changes made in struct net_device by commit >> cf124db566e6 ("net: Fix inconsistent teardown and release of private >> netdev state."). This only looks for instances that need >> free_netdev() call, ie. struct net_device::needs_free_netdev == true. > > I think we may need to worry about more kernel versions than just 4.12 > here, since this stuff is getting backported, and then it would cause > double-frees. > > I'm also considering simply removing the priv_destructor entirely from > mac80211, but I guess this would still apply to other drivers. For current backports the only other driver is brcmfmac. So you don't need it in mac80211? Does that mean you move stuff to .ndo_uninit()? Arend -- To unsubscribe from this list: send the line "unsubscribe backports" in
On Mon, 2017-06-12 at 11:15 +0200, Arend van Spriel wrote: > On 6/12/2017 11:11 AM, Johannes Berg wrote: > > On Mon, 2017-06-12 at 10:06 +0100, Arend van Spriel wrote: > > > This patch deals with changes made in struct net_device by commit > > > cf124db566e6 ("net: Fix inconsistent teardown and release of > > > private > > > netdev state."). This only looks for instances that need > > > free_netdev() call, ie. struct net_device::needs_free_netdev == > > > true. > > > > I think we may need to worry about more kernel versions than just > > 4.12 > > here, since this stuff is getting backported, and then it would > > cause > > double-frees. > > > > I'm also considering simply removing the priv_destructor entirely > > from > > mac80211, but I guess this would still apply to other drivers. > > For current backports the only other driver is brcmfmac. Right. > So you don't > need it in mac80211? Does that mean you move stuff to .ndo_uninit()? No, just call more things explicitly and remove destructor usage entirely. See the thread with davem? johannes -- To unsubscribe from this list: send the line "unsubscribe backports" in
On Mon, 2017-06-12 at 10:06 +0100, Arend van Spriel wrote: > This patch deals with changes made in struct net_device by commit > cf124db566e6 ("net: Fix inconsistent teardown and release of private > netdev state."). This only looks for instances that need > free_netdev() call, ie. struct net_device::needs_free_netdev == true. Come to think of it, isn't this missing the part where we now call priv_destructor when registering fails or something? johannes -- To unsubscribe from this list: send the line "unsubscribe backports" in
On 13-06-17 22:59, Johannes Berg wrote: > On Mon, 2017-06-12 at 10:06 +0100, Arend van Spriel wrote: >> This patch deals with changes made in struct net_device by commit >> cf124db566e6 ("net: Fix inconsistent teardown and release of private >> netdev state."). This only looks for instances that need >> free_netdev() call, ie. struct net_device::needs_free_netdev == true. > > Come to think of it, isn't this missing the part where we now call > priv_destructor when registering fails or something? Ah. I should have studied the patch better to see what behavioral changes the commit imposed. So are you still considering the patch despite the likely backport of commit cf124db566e6. I can do my homework better and resubmit if needed. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe backports" in
On Wed, 2017-06-14 at 15:25 +0200, Arend van Spriel wrote: > On 13-06-17 22:59, Johannes Berg wrote: > > On Mon, 2017-06-12 at 10:06 +0100, Arend van Spriel wrote: > > > This patch deals with changes made in struct net_device by commit > > > cf124db566e6 ("net: Fix inconsistent teardown and release of > > > private > > > netdev state."). This only looks for instances that need > > > free_netdev() call, ie. struct net_device::needs_free_netdev == > > > true. > > > > Come to think of it, isn't this missing the part where we now call > > priv_destructor when registering fails or something? > > Ah. I should have studied the patch better to see what behavioral > changes the commit imposed. Not sure, tbh. I just think there are issues there. > So are you still considering the patch > despite the likely backport of commit cf124db566e6. I can do my > homework better and resubmit if needed. Yeah I still think this backports patch makes sense, since I'm not sure I want to fix mac80211 that way (the bits in unregistering multiple netdevs are awkward) and brcmfmac would still need it. johannes -- To unsubscribe from this list: send the line "unsubscribe backports" in
diff --git a/backport/backport-include/linux/netdevice.h b/backport/backport-include/linux/netdevice.h index 06230b5..4e30383 100644 --- a/backport/backport-include/linux/netdevice.h +++ b/backport/backport-include/linux/netdevice.h @@ -320,4 +320,16 @@ static inline void netif_trans_update(struct net_device *dev) } #endif +static inline +void netdev_set_priv_destructor(struct net_device *dev, + void (*destructor)(struct net_device *dev)) +{ +#if LINUX_VERSION_IS_LESS(4,12,0) + dev->destructor = destructor; +#else + dev->needs_free_netdev = true; + dev->priv_destructor = destructor; +#endif +} + #endif /* __BACKPORT_NETDEVICE_H */ diff --git a/patches/0079-netdev-destructor.cocci b/patches/0079-netdev-destructor.cocci new file mode 100644 index 0000000..d8a439d --- /dev/null +++ b/patches/0079-netdev-destructor.cocci @@ -0,0 +1,33 @@ +@r1@ +struct net_device *ndev; +identifier D, C; +@@ +C(...) +{ + <... +- ndev->needs_free_netdev = true; +- ndev->priv_destructor = D; ++ netdev_set_priv_destructor(ndev, D); + ...> +} + +@r2 depends on r1@ +struct net_device *NDEV; +identifier r1.D; +identifier r1.C; +fresh identifier E2 = "__" ## D; +@@ + ++static void E2(struct net_device *dev) ++{ ++ D(dev); ++ free_netdev(dev); ++} ++ +C(...) +{ + <... +- netdev_set_priv_destructor(NDEV, D); ++ netdev_set_priv_destructor(NDEV, E2); + ...> +}
This patch deals with changes made in struct net_device by commit cf124db566e6 ("net: Fix inconsistent teardown and release of private netdev state."). This only looks for instances that need free_netdev() call, ie. struct net_device::needs_free_netdev == true. Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com> --- backport/backport-include/linux/netdevice.h | 12 +++++++++++ patches/0079-netdev-destructor.cocci | 33 +++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 patches/0079-netdev-destructor.cocci