On 14-06-17 15:26, Johannes Berg wrote:
> 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.
So I changed the semantic patch as below, which fixes the error path
when register_netdevice() fails:
@@ -1884,6 +1889,7 @@ int ieee80211_if_add(struct ieee80211_lo
ret = register_netdevice(ndev);
if (ret) {
+ ieee80211_if_free(ndev);
free_netdev(ndev);
return ret;
}
Unfortunately, it also creates the following hunk:
@@ -1779,6 +1783,7 @@ int ieee80211_if_add(struct ieee80211_lo
ndev->tstats = netdev_alloc_pcpu_stats(struct pcpu_sw_netstats);
if (!ndev->tstats) {
+ ieee80211_if_free(ndev);
free_netdev(ndev);
return -ENOMEM;
}
All ieee80211_if_free() does is freeing the tstats and it deals with
null pointers. So for mac80211 this is ok but not generically. Not sure
how to tackle this in a semantic patch.
Regards,
Arend
---
diff --git a/patches/0079-netdev-destructor.cocci
b/patches/0079-netdev-destructor.cocci
index d8a439d..71b5525 100644
--- a/patches/0079-netdev-destructor.cocci
+++ b/patches/0079-netdev-destructor.cocci
@@ -14,6 +14,15 @@ C(...)
@r2 depends on r1@
struct net_device *NDEV;
identifier r1.D;
+@@
+
+- free_netdev(NDEV);
++ D(NDEV);
++ free_netdev(NDEV);
+
+@r3 depends on r2@
+struct net_device *NDEV;
+identifier r1.D;
identifier r1.C;
fresh identifier E2 = "__" ## D;
@@
--
To unsubscribe from this list: send the line "unsubscribe backports" in
