| Message ID | 20230907025709.3409515-1-liujian56@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | ac28b1ec6135649b5d78b028e47264cb3ebca5ea |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] net: ipv4: fix one memleak in __inet_del_ifa() | expand |
Hello, On Thu, 7 Sep 2023, Liu Jian wrote: > I got the below warning when do fuzzing test: > unregister_netdevice: waiting for bond0 to become free. Usage count = 2 > > It can be repoduced via: > > ip link add bond0 type bond > sysctl -w net.ipv4.conf.bond0.promote_secondaries=1 > ip addr add 4.117.174.103/0 scope 0x40 dev bond0 > ip addr add 192.168.100.111/255.255.255.254 scope 0 dev bond0 > ip addr add 0.0.0.4/0 scope 0x40 secondary dev bond0 > ip addr del 4.117.174.103/0 scope 0x40 dev bond0 > ip link delete bond0 type bond > > In this reproduction test case, an incorrect 'last_prim' is found in > __inet_del_ifa(), as a result, the secondary address(0.0.0.4/0 scope 0x40) > is lost. The memory of the secondary address is leaked and the reference of > in_device and net_device is leaked. > > Fix this problem: > Look for 'last_prim' starting at location of the deleted IP and inserting > the promoted IP into the location of 'last_prim'. > > Fixes: 0ff60a45678e ("[IPV4]: Fix secondary IP addresses after promotion") > Signed-off-by: Liu Jian <liujian56@huawei.com> Looks good to me, thanks! Signed-off-by: Julian Anastasov <ja@ssi.bg> > --- > v1->v2: Change the implementation to Julian's. > The commit message is modified. > net/ipv4/devinet.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c > index 9cf64ee47dd2..ca0ff15dc8fa 100644 > --- a/net/ipv4/devinet.c > +++ b/net/ipv4/devinet.c > @@ -355,14 +355,14 @@ static void __inet_del_ifa(struct in_device *in_dev, > { > struct in_ifaddr *promote = NULL; > struct in_ifaddr *ifa, *ifa1; > - struct in_ifaddr *last_prim; > + struct in_ifaddr __rcu **last_prim; > struct in_ifaddr *prev_prom = NULL; > int do_promote = IN_DEV_PROMOTE_SECONDARIES(in_dev); > > ASSERT_RTNL(); > > ifa1 = rtnl_dereference(*ifap); > - last_prim = rtnl_dereference(in_dev->ifa_list); > + last_prim = ifap; > if (in_dev->dead) > goto no_promotions; > > @@ -376,7 +376,7 @@ static void __inet_del_ifa(struct in_device *in_dev, > while ((ifa = rtnl_dereference(*ifap1)) != NULL) { > if (!(ifa->ifa_flags & IFA_F_SECONDARY) && > ifa1->ifa_scope <= ifa->ifa_scope) > - last_prim = ifa; > + last_prim = &ifa->ifa_next; > > if (!(ifa->ifa_flags & IFA_F_SECONDARY) || > ifa1->ifa_mask != ifa->ifa_mask || > @@ -440,9 +440,9 @@ static void __inet_del_ifa(struct in_device *in_dev, > > rcu_assign_pointer(prev_prom->ifa_next, next_sec); > > - last_sec = rtnl_dereference(last_prim->ifa_next); > + last_sec = rtnl_dereference(*last_prim); > rcu_assign_pointer(promote->ifa_next, last_sec); > - rcu_assign_pointer(last_prim->ifa_next, promote); > + rcu_assign_pointer(*last_prim, promote); > } > > promote->ifa_flags &= ~IFA_F_SECONDARY; > -- > 2.34.1 Regards -- Julian Anastasov <ja@ssi.bg>
Hello: This patch was applied to netdev/net.git (main) by David S. Miller <davem@davemloft.net>: On Thu, 7 Sep 2023 10:57:09 +0800 you wrote: > I got the below warning when do fuzzing test: > unregister_netdevice: waiting for bond0 to become free. Usage count = 2 > > It can be repoduced via: > > ip link add bond0 type bond > sysctl -w net.ipv4.conf.bond0.promote_secondaries=1 > ip addr add 4.117.174.103/0 scope 0x40 dev bond0 > ip addr add 192.168.100.111/255.255.255.254 scope 0 dev bond0 > ip addr add 0.0.0.4/0 scope 0x40 secondary dev bond0 > ip addr del 4.117.174.103/0 scope 0x40 dev bond0 > ip link delete bond0 type bond > > [...] Here is the summary with links: - [net,v2] net: ipv4: fix one memleak in __inet_del_ifa() https://git.kernel.org/netdev/net/c/ac28b1ec6135 You are awesome, thank you!
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c index 9cf64ee47dd2..ca0ff15dc8fa 100644 --- a/net/ipv4/devinet.c +++ b/net/ipv4/devinet.c @@ -355,14 +355,14 @@ static void __inet_del_ifa(struct in_device *in_dev, { struct in_ifaddr *promote = NULL; struct in_ifaddr *ifa, *ifa1; - struct in_ifaddr *last_prim; + struct in_ifaddr __rcu **last_prim; struct in_ifaddr *prev_prom = NULL; int do_promote = IN_DEV_PROMOTE_SECONDARIES(in_dev); ASSERT_RTNL(); ifa1 = rtnl_dereference(*ifap); - last_prim = rtnl_dereference(in_dev->ifa_list); + last_prim = ifap; if (in_dev->dead) goto no_promotions; @@ -376,7 +376,7 @@ static void __inet_del_ifa(struct in_device *in_dev, while ((ifa = rtnl_dereference(*ifap1)) != NULL) { if (!(ifa->ifa_flags & IFA_F_SECONDARY) && ifa1->ifa_scope <= ifa->ifa_scope) - last_prim = ifa; + last_prim = &ifa->ifa_next; if (!(ifa->ifa_flags & IFA_F_SECONDARY) || ifa1->ifa_mask != ifa->ifa_mask || @@ -440,9 +440,9 @@ static void __inet_del_ifa(struct in_device *in_dev, rcu_assign_pointer(prev_prom->ifa_next, next_sec); - last_sec = rtnl_dereference(last_prim->ifa_next); + last_sec = rtnl_dereference(*last_prim); rcu_assign_pointer(promote->ifa_next, last_sec); - rcu_assign_pointer(last_prim->ifa_next, promote); + rcu_assign_pointer(*last_prim, promote); } promote->ifa_flags &= ~IFA_F_SECONDARY;
I got the below warning when do fuzzing test: unregister_netdevice: waiting for bond0 to become free. Usage count = 2 It can be repoduced via: ip link add bond0 type bond sysctl -w net.ipv4.conf.bond0.promote_secondaries=1 ip addr add 4.117.174.103/0 scope 0x40 dev bond0 ip addr add 192.168.100.111/255.255.255.254 scope 0 dev bond0 ip addr add 0.0.0.4/0 scope 0x40 secondary dev bond0 ip addr del 4.117.174.103/0 scope 0x40 dev bond0 ip link delete bond0 type bond In this reproduction test case, an incorrect 'last_prim' is found in __inet_del_ifa(), as a result, the secondary address(0.0.0.4/0 scope 0x40) is lost. The memory of the secondary address is leaked and the reference of in_device and net_device is leaked. Fix this problem: Look for 'last_prim' starting at location of the deleted IP and inserting the promoted IP into the location of 'last_prim'. Fixes: 0ff60a45678e ("[IPV4]: Fix secondary IP addresses after promotion") Signed-off-by: Liu Jian <liujian56@huawei.com> --- v1->v2: Change the implementation to Julian's. The commit message is modified. net/ipv4/devinet.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)