[PATCH net-next 0/2] net: ipv4: fix circular-list infinite loop

[PATCH net-next 0/2] net: ipv4: fix circular-list infinite loop

[Florian Westphal]

Tariq and Ran reported a regression caused by net-next commit
2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list").

This happens when net.ipv4.conf.$dev.promote_secondaries sysctl is
enabled -- we can arrange for ifa->next to point at ifa, so next
process that tries to walk the list loops forever.

Fix this and extend rtnetlink.sh with a small test case for this.

Florian Westphal (2):
      net: ipv4: fix infinite loop on secondary addr promotion
      selftests: rtnetlink: add small test case with 'promote_secondaries' enabled

 net/ipv4/devinet.c                       |    3 ++-
 tools/testing/selftests/net/rtnetlink.sh |   20 ++++++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

[PATCH net-next 1/2] net: ipv4: fix infinite loop on secondary addr promotion

[Florian Westphal]

secondary address promotion causes infinite loop -- it arranges
for ifa->ifa_next to point back to itself.

Problem is that 'prev_prom' and 'last_prim' might point at the same entry,
so 'last_sec' pointer must be obtained after prev_prom->next update.

Fixes: 2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list")
Reported-by: Ran Rozenstein <ranro@mellanox.com>
Reported-by: Tariq Toukan <tariqt@mellanox.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/ipv4/devinet.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 7874303220c5..137d1892395d 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -428,8 +428,9 @@ static void __inet_del_ifa(struct in_device *in_dev,
 		if (prev_prom) {
 			struct in_ifaddr *last_sec;
 
-			last_sec = rtnl_dereference(last_prim->ifa_next);
 			rcu_assign_pointer(prev_prom->ifa_next, next_sec);
+
+			last_sec = rtnl_dereference(last_prim->ifa_next);
 			rcu_assign_pointer(promote->ifa_next, last_sec);
 			rcu_assign_pointer(last_prim->ifa_next, promote);
 		}
-- 
2.21.0

[PATCH net-next 2/2] selftests: rtnetlink: add small test case with 'promote_secondaries' enabled

[Florian Westphal]

This exercises the 'promote_secondaries' code path.

Without previous fix, this triggers infinite loop/soft lockup:
ifconfig process spinning at 100%, never to return.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 tools/testing/selftests/net/rtnetlink.sh | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index ed606a2e3865..505628884783 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh
@@ -269,6 +269,25 @@ kci_test_addrlft()
 	echo "PASS: preferred_lft addresses have expired"
 }
 
+kci_test_promote_secondaries()
+{
+	promote=$(sysctl -n net.ipv4.conf.$devdummy.promote_secondaries)
+
+	sysctl -q net.ipv4.conf.$devdummy.promote_secondaries=1
+
+	for i in $(seq 2 254);do
+		IP="10.23.11.$i"
+		ip -f inet addr add $IP/16 brd + dev "$devdummy"
+		ifconfig "$devdummy" $IP netmask 255.255.0.0
+	done
+
+	ip addr flush dev "$devdummy"
+
+	[ $promote -eq 0 ] && sysctl -q net.ipv4.conf.$devdummy.promote_secondaries=0
+
+	echo "PASS: promote_secondaries complete"
+}
+
 kci_test_addrlabel()
 {
 	ret=0
@@ -1161,6 +1180,7 @@ kci_test_rtnl()
 	kci_test_polrouting
 	kci_test_route_get
 	kci_test_addrlft
+	kci_test_promote_secondaries
 	kci_test_tc
 	kci_test_gre
 	kci_test_gretap
-- 
2.21.0

Re: [PATCH net-next 0/2] net: ipv4: fix circular-list infinite loop

[David Miller]

From: Florian Westphal <fw@strlen.de>
Date: Thu, 27 Jun 2019 14:03:31 +0200

> Tariq and Ran reported a regression caused by net-next commit
> 2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list").
> 
> This happens when net.ipv4.conf.$dev.promote_secondaries sysctl is
> enabled -- we can arrange for ifa->next to point at ifa, so next
> process that tries to walk the list loops forever.
> 
> Fix this and extend rtnetlink.sh with a small test case for this.

Series applied, thanks Florian.

Re: [PATCH net-next 0/2] net: ipv4: fix circular-list infinite loop

[Tariq Toukan]

On 6/27/2019 7:54 PM, David Miller wrote:
> From: Florian Westphal <fw@strlen.de>
> Date: Thu, 27 Jun 2019 14:03:31 +0200
> 
>> Tariq and Ran reported a regression caused by net-next commit
>> 2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list").
>>
>> This happens when net.ipv4.conf.$dev.promote_secondaries sysctl is
>> enabled -- we can arrange for ifa->next to point at ifa, so next
>> process that tries to walk the list loops forever.
>>
>> Fix this and extend rtnetlink.sh with a small test case for this.
> 
> Series applied, thanks Florian.
> 

Thanks Florian!

Ran, please test and update.

Tariq

RE: [PATCH net-next 0/2] net: ipv4: fix circular-list infinite loop

[Ran Rozenstein]

> -----Original Message-----
> From: Tariq Toukan
> Sent: Sunday, June 30, 2019 10:57
> To: David Miller <davem@davemloft.net>; fw@strlen.de
> Cc: netdev@vger.kernel.org; Ran Rozenstein <ranro@mellanox.com>; Tariq
> Toukan <tariqt@mellanox.com>
> Subject: Re: [PATCH net-next 0/2] net: ipv4: fix circular-list infinite loop
> 
> 
> 
> On 6/27/2019 7:54 PM, David Miller wrote:
> > From: Florian Westphal <fw@strlen.de>
> > Date: Thu, 27 Jun 2019 14:03:31 +0200
> >
> >> Tariq and Ran reported a regression caused by net-next commit
> >> 2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list").
> >>
> >> This happens when net.ipv4.conf.$dev.promote_secondaries sysctl is
> >> enabled -- we can arrange for ifa->next to point at ifa, so next
> >> process that tries to walk the list loops forever.
> >>
> >> Fix this and extend rtnetlink.sh with a small test case for this.
> >
> > Series applied, thanks Florian.
> >
> 
> Thanks Florian!
> 
> Ran, please test and update.
> 
> Tariq

Thanks Florian.
Didn't reproduce tonight with the fixes.

Ran.

Re: [PATCH net-next 0/2] net: ipv4: fix circular-list infinite loop

[Tariq Toukan]

On 7/1/2019 9:46 AM, Ran Rozenstein wrote:
> 
> 
>> -----Original Message-----
>> From: Tariq Toukan
>> Sent: Sunday, June 30, 2019 10:57
>> To: David Miller <davem@davemloft.net>; fw@strlen.de
>> Cc: netdev@vger.kernel.org; Ran Rozenstein <ranro@mellanox.com>; Tariq
>> Toukan <tariqt@mellanox.com>
>> Subject: Re: [PATCH net-next 0/2] net: ipv4: fix circular-list infinite loop
>>
>>
>>
>> On 6/27/2019 7:54 PM, David Miller wrote:
>>> From: Florian Westphal <fw@strlen.de>
>>> Date: Thu, 27 Jun 2019 14:03:31 +0200
>>>
>>>> Tariq and Ran reported a regression caused by net-next commit
>>>> 2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list").
>>>>
>>>> This happens when net.ipv4.conf.$dev.promote_secondaries sysctl is
>>>> enabled -- we can arrange for ifa->next to point at ifa, so next
>>>> process that tries to walk the list loops forever.
>>>>
>>>> Fix this and extend rtnetlink.sh with a small test case for this.
>>>
>>> Series applied, thanks Florian.
>>>
>>
>> Thanks Florian!
>>
>> Ran, please test and update.
>>
>> Tariq
> 
> Thanks Florian.
> Didn't reproduce tonight with the fixes.
> 
> Ran.
> 

Sounds good!

Thanks,
Tariq