* [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer
@ 2019-06-17 14:02 Florian Westphal
2019-06-17 14:02 ` [PATCH net-next 1/2] " Florian Westphal
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Florian Westphal @ 2019-06-17 14:02 UTC (permalink / raw)
To: netdev; +Cc: tariqt, ranro, maorg, edumazet
Tariq reported a soft lockup on net-next that Mellanox was able to
bisect to 2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list").
While reviewing above patch I found a regression when addresses have a
lifetime specified.
Second patch extends rtnetlink.sh to trigger crash
(without first patch applied).
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net-next 1/2] net: ipv4: remove erroneous advancement of list pointer
2019-06-17 14:02 [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer Florian Westphal
@ 2019-06-17 14:02 ` Florian Westphal
2019-06-17 14:02 ` [PATCH net-next 2/2] selftests: rtnetlink: add addresses with fixed life time Florian Westphal
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Florian Westphal @ 2019-06-17 14:02 UTC (permalink / raw)
To: netdev; +Cc: tariqt, ranro, maorg, edumazet, Florian Westphal
Causes crash when lifetime expires on an adress as garbage is
dereferenced soon after.
This used to look like this:
for (ifap = &ifa->ifa_dev->ifa_list;
*ifap != NULL; ifap = &(*ifap)->ifa_next) {
if (*ifap == ifa) ...
but this was changed to:
struct in_ifaddr *tmp;
ifap = &ifa->ifa_dev->ifa_list;
tmp = rtnl_dereference(*ifap);
while (tmp) {
tmp = rtnl_dereference(tmp->ifa_next); // Bogus
if (rtnl_dereference(*ifap) == ifa) {
...
ifap = &tmp->ifa_next; // Can be NULL
tmp = rtnl_dereference(*ifap); // Dereference
}
}
Remove the bogus assigment/list entry skip.
Fixes: 2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/ipv4/devinet.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 925dffa915cb..914ccc7f192a 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -745,8 +745,7 @@ static void check_lifetime(struct work_struct *work)
ifap = &ifa->ifa_dev->ifa_list;
tmp = rtnl_dereference(*ifap);
while (tmp) {
- tmp = rtnl_dereference(tmp->ifa_next);
- if (rtnl_dereference(*ifap) == ifa) {
+ if (tmp == ifa) {
inet_del_ifa(ifa->ifa_dev,
ifap, 1);
break;
--
2.21.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH net-next 2/2] selftests: rtnetlink: add addresses with fixed life time
2019-06-17 14:02 [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer Florian Westphal
2019-06-17 14:02 ` [PATCH net-next 1/2] " Florian Westphal
@ 2019-06-17 14:02 ` Florian Westphal
2019-06-17 16:16 ` [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer Tariq Toukan
2019-06-17 23:27 ` David Miller
3 siblings, 0 replies; 10+ messages in thread
From: Florian Westphal @ 2019-06-17 14:02 UTC (permalink / raw)
To: netdev; +Cc: tariqt, ranro, maorg, edumazet, Florian Westphal
This exercises kernel code path that deal with addresses that have
a limited lifetime.
Without previous fix, this triggers following crash on net-next:
BUG: KASAN: null-ptr-deref in check_lifetime+0x403/0x670
Read of size 8 at addr 0000000000000010 by task kworker [..]
Signed-off-by: Florian Westphal <fw@strlen.de>
---
tools/testing/selftests/net/rtnetlink.sh | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index b25c9fe019d2..ed606a2e3865 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh
@@ -249,6 +249,26 @@ kci_test_route_get()
echo "PASS: route get"
}
+kci_test_addrlft()
+{
+ for i in $(seq 10 100) ;do
+ lft=$(((RANDOM%3) + 1))
+ ip addr add 10.23.11.$i/32 dev "$devdummy" preferred_lft $lft valid_lft $((lft+1))
+ check_err $?
+ done
+
+ sleep 5
+
+ ip addr show dev "$devdummy" | grep "10.23.11."
+ if [ $? -eq 0 ]; then
+ echo "FAIL: preferred_lft addresses remaining"
+ check_err 1
+ return
+ fi
+
+ echo "PASS: preferred_lft addresses have expired"
+}
+
kci_test_addrlabel()
{
ret=0
@@ -1140,6 +1160,7 @@ kci_test_rtnl()
kci_test_polrouting
kci_test_route_get
+ kci_test_addrlft
kci_test_tc
kci_test_gre
kci_test_gretap
--
2.21.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer
2019-06-17 14:02 [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer Florian Westphal
2019-06-17 14:02 ` [PATCH net-next 1/2] " Florian Westphal
2019-06-17 14:02 ` [PATCH net-next 2/2] selftests: rtnetlink: add addresses with fixed life time Florian Westphal
@ 2019-06-17 16:16 ` Tariq Toukan
2019-06-25 9:04 ` Ran Rozenstein
2019-06-17 23:27 ` David Miller
3 siblings, 1 reply; 10+ messages in thread
From: Tariq Toukan @ 2019-06-17 16:16 UTC (permalink / raw)
To: Florian Westphal, netdev; +Cc: Ran Rozenstein, Maor Gottlieb, edumazet
On 6/17/2019 5:02 PM, Florian Westphal wrote:
> Tariq reported a soft lockup on net-next that Mellanox was able to
> bisect to 2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list").
>
> While reviewing above patch I found a regression when addresses have a
> lifetime specified.
>
> Second patch extends rtnetlink.sh to trigger crash
> (without first patch applied).
>
Thanks Florian.
Ran, can you please test?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer
2019-06-17 14:02 [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer Florian Westphal
` (2 preceding siblings ...)
2019-06-17 16:16 ` [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer Tariq Toukan
@ 2019-06-17 23:27 ` David Miller
3 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2019-06-17 23:27 UTC (permalink / raw)
To: fw; +Cc: netdev, tariqt, ranro, maorg, edumazet
From: Florian Westphal <fw@strlen.de>
Date: Mon, 17 Jun 2019 16:02:26 +0200
> Tariq reported a soft lockup on net-next that Mellanox was able to
> bisect to 2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list").
>
> While reviewing above patch I found a regression when addresses have a
> lifetime specified.
>
> Second patch extends rtnetlink.sh to trigger crash
> (without first patch applied).
Series applied, thanks Florian.
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer
2019-06-17 16:16 ` [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer Tariq Toukan
@ 2019-06-25 9:04 ` Ran Rozenstein
2019-06-25 9:19 ` Florian Westphal
0 siblings, 1 reply; 10+ messages in thread
From: Ran Rozenstein @ 2019-06-25 9:04 UTC (permalink / raw)
To: Tariq Toukan, Florian Westphal, netdev; +Cc: Maor Gottlieb, edumazet
> -----Original Message-----
> From: Tariq Toukan
> Sent: Monday, June 17, 2019 19:16
> To: Florian Westphal <fw@strlen.de>; netdev@vger.kernel.org
> Cc: Ran Rozenstein <ranro@mellanox.com>; Maor Gottlieb
> <maorg@mellanox.com>; edumazet@google.com
> Subject: Re: [PATCH net-next 0/2] net: ipv4: remove erroneous
> advancement of list pointer
>
>
>
> On 6/17/2019 5:02 PM, Florian Westphal wrote:
> > Tariq reported a soft lockup on net-next that Mellanox was able to
> > bisect to 2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list").
> >
> > While reviewing above patch I found a regression when addresses have a
> > lifetime specified.
> >
> > Second patch extends rtnetlink.sh to trigger crash (without first
> > patch applied).
> >
>
> Thanks Florian.
>
> Ran, can you please test?
Tested, still reproduce.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer
2019-06-25 9:04 ` Ran Rozenstein
@ 2019-06-25 9:19 ` Florian Westphal
2019-06-26 12:48 ` Ran Rozenstein
0 siblings, 1 reply; 10+ messages in thread
From: Florian Westphal @ 2019-06-25 9:19 UTC (permalink / raw)
To: Ran Rozenstein
Cc: Tariq Toukan, Florian Westphal, netdev, Maor Gottlieb, edumazet
Ran Rozenstein <ranro@mellanox.com> wrote:
> > On 6/17/2019 5:02 PM, Florian Westphal wrote:
> > > Tariq reported a soft lockup on net-next that Mellanox was able to
> > > bisect to 2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list").
> > >
> > > While reviewing above patch I found a regression when addresses have a
> > > lifetime specified.
> > >
> > > Second patch extends rtnetlink.sh to trigger crash (without first
> > > patch applied).
> > >
> >
> > Thanks Florian.
> >
> > Ran, can you please test?
>
> Tested, still reproduce.
Can you be a little more specific? Is there any reproducer?
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer
2019-06-25 9:19 ` Florian Westphal
@ 2019-06-26 12:48 ` Ran Rozenstein
2019-06-26 20:37 ` Florian Westphal
2019-06-26 23:32 ` Florian Westphal
0 siblings, 2 replies; 10+ messages in thread
From: Ran Rozenstein @ 2019-06-26 12:48 UTC (permalink / raw)
To: Florian Westphal; +Cc: Tariq Toukan, netdev, Maor Gottlieb, edumazet
> -----Original Message-----
> From: Florian Westphal [mailto:fw@strlen.de]
> Sent: Tuesday, June 25, 2019 12:19
> To: Ran Rozenstein <ranro@mellanox.com>
> Cc: Tariq Toukan <tariqt@mellanox.com>; Florian Westphal <fw@strlen.de>;
> netdev@vger.kernel.org; Maor Gottlieb <maorg@mellanox.com>;
> edumazet@google.com
> Subject: Re: [PATCH net-next 0/2] net: ipv4: remove erroneous
> advancement of list pointer
>
> Ran Rozenstein <ranro@mellanox.com> wrote:
> > > On 6/17/2019 5:02 PM, Florian Westphal wrote:
> > > > Tariq reported a soft lockup on net-next that Mellanox was able to
> > > > bisect to 2638eb8b50cf ("net: ipv4: provide __rcu annotation for
> ifa_list").
> > > >
> > > > While reviewing above patch I found a regression when addresses
> > > > have a lifetime specified.
> > > >
> > > > Second patch extends rtnetlink.sh to trigger crash (without first
> > > > patch applied).
> > > >
> > >
> > > Thanks Florian.
> > >
> > > Ran, can you please test?
> >
> > Tested, still reproduce.
>
> Can you be a little more specific? Is there any reproducer?
The test dose stress on the interface by running this 2 commands in loop:
command is: /sbin/ip -f inet addr add $IP/16 brd + dev ens8f1
command is: ifconfig ens8f1 $IP netmask 255.255.0.0
when $IP change every iteration.
It execute every second when we see the reproduce somewhere between 40 to 200 seconds of execution.
Thanks,
Ran
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer
2019-06-26 12:48 ` Ran Rozenstein
@ 2019-06-26 20:37 ` Florian Westphal
2019-06-26 23:32 ` Florian Westphal
1 sibling, 0 replies; 10+ messages in thread
From: Florian Westphal @ 2019-06-26 20:37 UTC (permalink / raw)
To: Ran Rozenstein
Cc: Florian Westphal, Tariq Toukan, netdev, Maor Gottlieb, edumazet
Ran Rozenstein <ranro@mellanox.com> wrote:
> The test dose stress on the interface by running this 2 commands in loop:
>
> command is: /sbin/ip -f inet addr add $IP/16 brd + dev ens8f1
> command is: ifconfig ens8f1 $IP netmask 255.255.0.0
>
> when $IP change every iteration.
>
> It execute every second when we see the reproduce somewhere between 40 to 200 seconds of execution.
I tried this without success:
DEV=dummy0
for j in $(seq 2 254);do
for i in $(seq 2 254);do
IP="10.$((RANDOM%254)).$((RANDOM%254)).$i"
ip -f inet addr add $IP/16 brd + dev $DEV
ifconfig $DEV $IP netmask 255.255.0.0
done
done
I'll let this loop overnight, but so far nothing turned up in
dmesg (lockdep/kasan kernel).
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer
2019-06-26 12:48 ` Ran Rozenstein
2019-06-26 20:37 ` Florian Westphal
@ 2019-06-26 23:32 ` Florian Westphal
1 sibling, 0 replies; 10+ messages in thread
From: Florian Westphal @ 2019-06-26 23:32 UTC (permalink / raw)
To: Ran Rozenstein
Cc: Florian Westphal, Tariq Toukan, netdev, Maor Gottlieb, edumazet
Ran Rozenstein <ranro@mellanox.com> wrote:
> The test dose stress on the interface by running this 2 commands in loop:
>
> command is: /sbin/ip -f inet addr add $IP/16 brd + dev ens8f1
> command is: ifconfig ens8f1 $IP netmask 255.255.0.0
>
> when $IP change every iteration.
>
> It execute every second when we see the reproduce somewhere between 40 to 200 seconds of execution.
I can reproduce this now, I'll submit a fix tomorrow.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-06-26 23:32 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-17 14:02 [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer Florian Westphal
2019-06-17 14:02 ` [PATCH net-next 1/2] " Florian Westphal
2019-06-17 14:02 ` [PATCH net-next 2/2] selftests: rtnetlink: add addresses with fixed life time Florian Westphal
2019-06-17 16:16 ` [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer Tariq Toukan
2019-06-25 9:04 ` Ran Rozenstein
2019-06-25 9:19 ` Florian Westphal
2019-06-26 12:48 ` Ran Rozenstein
2019-06-26 20:37 ` Florian Westphal
2019-06-26 23:32 ` Florian Westphal
2019-06-17 23:27 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).