* [PATCH net 0/2] ipmr: Always call ip{,6}_mr_forward() from RCU read-side critical section
@ 2022-09-14 7:53 Ido Schimmel
2022-09-14 7:53 ` [PATCH net 1/2] " Ido Schimmel
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Ido Schimmel @ 2022-09-14 7:53 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, pabeni, edumazet, dsahern, mlxsw, Ido Schimmel
Patch #1 fixes a bug in ipmr code.
Patch #2 adds corresponding test cases.
Ido Schimmel (2):
ipmr: Always call ip{,6}_mr_forward() from RCU read-side critical
section
selftests: forwarding: Add test cases for unresolved multicast routes
net/ipv4/ipmr.c | 2 +
net/ipv6/ip6mr.c | 5 +-
.../net/forwarding/router_multicast.sh | 92 ++++++++++++++++++-
3 files changed, 97 insertions(+), 2 deletions(-)
--
2.37.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH net 1/2] ipmr: Always call ip{,6}_mr_forward() from RCU read-side critical section
2022-09-14 7:53 [PATCH net 0/2] ipmr: Always call ip{,6}_mr_forward() from RCU read-side critical section Ido Schimmel
@ 2022-09-14 7:53 ` Ido Schimmel
2022-09-14 7:53 ` [PATCH net 2/2] selftests: forwarding: Add test cases for unresolved multicast routes Ido Schimmel
2022-09-20 15:40 ` [PATCH net 0/2] ipmr: Always call ip{,6}_mr_forward() from RCU read-side critical section patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: Ido Schimmel @ 2022-09-14 7:53 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, pabeni, edumazet, dsahern, mlxsw, Ido Schimmel
These functions expect to be called from RCU read-side critical section,
but this only happens when invoked from the data path via
ip{,6}_mr_input(). They can also be invoked from process context in
response to user space adding a multicast route which resolves a cache
entry with queued packets [1][2].
Fix by adding missing rcu_read_lock() / rcu_read_unlock() in these call
paths.
[1]
WARNING: suspicious RCU usage
6.0.0-rc3-custom-15969-g049d233c8bcc-dirty #1387 Not tainted
-----------------------------
net/ipv4/ipmr.c:84 suspicious rcu_dereference_check() usage!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
1 lock held by smcrouted/246:
#0: ffffffff862389b0 (rtnl_mutex){+.+.}-{3:3}, at: ip_mroute_setsockopt+0x11c/0x1420
stack backtrace:
CPU: 0 PID: 246 Comm: smcrouted Not tainted 6.0.0-rc3-custom-15969-g049d233c8bcc-dirty #1387
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-1.fc36 04/01/2014
Call Trace:
<TASK>
dump_stack_lvl+0x91/0xb9
vif_dev_read+0xbf/0xd0
ipmr_queue_xmit+0x135/0x1ab0
ip_mr_forward+0xe7b/0x13d0
ipmr_mfc_add+0x1a06/0x2ad0
ip_mroute_setsockopt+0x5c1/0x1420
do_ip_setsockopt+0x23d/0x37f0
ip_setsockopt+0x56/0x80
raw_setsockopt+0x219/0x290
__sys_setsockopt+0x236/0x4d0
__x64_sys_setsockopt+0xbe/0x160
do_syscall_64+0x34/0x80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
[2]
WARNING: suspicious RCU usage
6.0.0-rc3-custom-15969-g049d233c8bcc-dirty #1387 Not tainted
-----------------------------
net/ipv6/ip6mr.c:69 suspicious rcu_dereference_check() usage!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
1 lock held by smcrouted/246:
#0: ffffffff862389b0 (rtnl_mutex){+.+.}-{3:3}, at: ip6_mroute_setsockopt+0x6b9/0x2630
stack backtrace:
CPU: 1 PID: 246 Comm: smcrouted Not tainted 6.0.0-rc3-custom-15969-g049d233c8bcc-dirty #1387
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-1.fc36 04/01/2014
Call Trace:
<TASK>
dump_stack_lvl+0x91/0xb9
vif_dev_read+0xbf/0xd0
ip6mr_forward2.isra.0+0xc9/0x1160
ip6_mr_forward+0xef0/0x13f0
ip6mr_mfc_add+0x1ff2/0x31f0
ip6_mroute_setsockopt+0x1825/0x2630
do_ipv6_setsockopt+0x462/0x4440
ipv6_setsockopt+0x105/0x140
rawv6_setsockopt+0xd8/0x690
__sys_setsockopt+0x236/0x4d0
__x64_sys_setsockopt+0xbe/0x160
do_syscall_64+0x34/0x80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
Fixes: ebc3197963fc ("ipmr: add rcu protection over (struct vif_device)->dev")
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
net/ipv4/ipmr.c | 2 ++
net/ipv6/ip6mr.c | 5 ++++-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 73651d17e51f..e11d6b0b62b7 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1004,7 +1004,9 @@ static void ipmr_cache_resolve(struct net *net, struct mr_table *mrt,
rtnl_unicast(skb, net, NETLINK_CB(skb).portid);
} else {
+ rcu_read_lock();
ip_mr_forward(net, mrt, skb->dev, skb, c, 0);
+ rcu_read_unlock();
}
}
}
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index a9ba41648e36..858fd8a28b5b 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -1028,8 +1028,11 @@ static void ip6mr_cache_resolve(struct net *net, struct mr_table *mrt,
((struct nlmsgerr *)nlmsg_data(nlh))->error = -EMSGSIZE;
}
rtnl_unicast(skb, net, NETLINK_CB(skb).portid);
- } else
+ } else {
+ rcu_read_lock();
ip6_mr_forward(net, mrt, skb->dev, skb, c);
+ rcu_read_unlock();
+ }
}
}
--
2.37.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH net 2/2] selftests: forwarding: Add test cases for unresolved multicast routes
2022-09-14 7:53 [PATCH net 0/2] ipmr: Always call ip{,6}_mr_forward() from RCU read-side critical section Ido Schimmel
2022-09-14 7:53 ` [PATCH net 1/2] " Ido Schimmel
@ 2022-09-14 7:53 ` Ido Schimmel
2022-09-20 15:40 ` [PATCH net 0/2] ipmr: Always call ip{,6}_mr_forward() from RCU read-side critical section patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: Ido Schimmel @ 2022-09-14 7:53 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, pabeni, edumazet, dsahern, mlxsw, Ido Schimmel
Add IPv4 and IPv6 test cases for unresolved multicast routes, testing
that queued packets are forwarded after installing a matching (S, G)
route.
The test cases can be used to reproduce the bugs fixed in "ipmr: Always
call ip{,6}_mr_forward() from RCU read-side critical section".
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
.../net/forwarding/router_multicast.sh | 92 ++++++++++++++++++-
1 file changed, 91 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/forwarding/router_multicast.sh b/tools/testing/selftests/net/forwarding/router_multicast.sh
index 57e90c873a2c..5a58b1ec8aef 100755
--- a/tools/testing/selftests/net/forwarding/router_multicast.sh
+++ b/tools/testing/selftests/net/forwarding/router_multicast.sh
@@ -28,7 +28,7 @@
# +------------------+ +------------------+
#
-ALL_TESTS="mcast_v4 mcast_v6 rpf_v4 rpf_v6"
+ALL_TESTS="mcast_v4 mcast_v6 rpf_v4 rpf_v6 unres_v4 unres_v6"
NUM_NETIFS=6
source lib.sh
source tc_common.sh
@@ -406,6 +406,96 @@ rpf_v6()
log_test "RPF IPv6"
}
+unres_v4()
+{
+ # Send a multicast packet not corresponding to an installed route,
+ # causing the kernel to queue the packet for resolution and emit an
+ # IGMPMSG_NOCACHE notification. smcrouted will react to this
+ # notification by consulting its (*, G) list and installing an (S, G)
+ # route, which will be used to forward the queued packet.
+
+ RET=0
+
+ tc filter add dev $h2 ingress protocol ip pref 1 handle 1 flower \
+ dst_ip 225.1.2.3 ip_proto udp dst_port 12345 action drop
+ tc filter add dev $h3 ingress protocol ip pref 1 handle 1 flower \
+ dst_ip 225.1.2.3 ip_proto udp dst_port 12345 action drop
+
+ # Forwarding should fail before installing a matching (*, G).
+ $MZ $h1 -c 1 -p 128 -t udp "ttl=10,sp=54321,dp=12345" \
+ -a 00:11:22:33:44:55 -b 01:00:5e:01:02:03 \
+ -A 198.51.100.2 -B 225.1.2.3 -q
+
+ tc_check_packets "dev $h2 ingress" 1 0
+ check_err $? "Multicast received on first host when should not"
+ tc_check_packets "dev $h3 ingress" 1 0
+ check_err $? "Multicast received on second host when should not"
+
+ # Create (*, G). Will not be installed in the kernel.
+ create_mcast_sg $rp1 0.0.0.0 225.1.2.3 $rp2 $rp3
+
+ $MZ $h1 -c 1 -p 128 -t udp "ttl=10,sp=54321,dp=12345" \
+ -a 00:11:22:33:44:55 -b 01:00:5e:01:02:03 \
+ -A 198.51.100.2 -B 225.1.2.3 -q
+
+ tc_check_packets "dev $h2 ingress" 1 1
+ check_err $? "Multicast not received on first host"
+ tc_check_packets "dev $h3 ingress" 1 1
+ check_err $? "Multicast not received on second host"
+
+ delete_mcast_sg $rp1 0.0.0.0 225.1.2.3 $rp2 $rp3
+
+ tc filter del dev $h3 ingress protocol ip pref 1 handle 1 flower
+ tc filter del dev $h2 ingress protocol ip pref 1 handle 1 flower
+
+ log_test "Unresolved queue IPv4"
+}
+
+unres_v6()
+{
+ # Send a multicast packet not corresponding to an installed route,
+ # causing the kernel to queue the packet for resolution and emit an
+ # MRT6MSG_NOCACHE notification. smcrouted will react to this
+ # notification by consulting its (*, G) list and installing an (S, G)
+ # route, which will be used to forward the queued packet.
+
+ RET=0
+
+ tc filter add dev $h2 ingress protocol ipv6 pref 1 handle 1 flower \
+ dst_ip ff0e::3 ip_proto udp dst_port 12345 action drop
+ tc filter add dev $h3 ingress protocol ipv6 pref 1 handle 1 flower \
+ dst_ip ff0e::3 ip_proto udp dst_port 12345 action drop
+
+ # Forwarding should fail before installing a matching (*, G).
+ $MZ $h1 -6 -c 1 -p 128 -t udp "ttl=10,sp=54321,dp=12345" \
+ -a 00:11:22:33:44:55 -b 33:33:00:00:00:03 \
+ -A 2001:db8:1::2 -B ff0e::3 -q
+
+ tc_check_packets "dev $h2 ingress" 1 0
+ check_err $? "Multicast received on first host when should not"
+ tc_check_packets "dev $h3 ingress" 1 0
+ check_err $? "Multicast received on second host when should not"
+
+ # Create (*, G). Will not be installed in the kernel.
+ create_mcast_sg $rp1 :: ff0e::3 $rp2 $rp3
+
+ $MZ $h1 -6 -c 1 -p 128 -t udp "ttl=10,sp=54321,dp=12345" \
+ -a 00:11:22:33:44:55 -b 33:33:00:00:00:03 \
+ -A 2001:db8:1::2 -B ff0e::3 -q
+
+ tc_check_packets "dev $h2 ingress" 1 1
+ check_err $? "Multicast not received on first host"
+ tc_check_packets "dev $h3 ingress" 1 1
+ check_err $? "Multicast not received on second host"
+
+ delete_mcast_sg $rp1 :: ff0e::3 $rp2 $rp3
+
+ tc filter del dev $h3 ingress protocol ipv6 pref 1 handle 1 flower
+ tc filter del dev $h2 ingress protocol ipv6 pref 1 handle 1 flower
+
+ log_test "Unresolved queue IPv6"
+}
+
trap cleanup EXIT
setup_prepare
--
2.37.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net 0/2] ipmr: Always call ip{,6}_mr_forward() from RCU read-side critical section
2022-09-14 7:53 [PATCH net 0/2] ipmr: Always call ip{,6}_mr_forward() from RCU read-side critical section Ido Schimmel
2022-09-14 7:53 ` [PATCH net 1/2] " Ido Schimmel
2022-09-14 7:53 ` [PATCH net 2/2] selftests: forwarding: Add test cases for unresolved multicast routes Ido Schimmel
@ 2022-09-20 15:40 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-09-20 15:40 UTC (permalink / raw)
To: Ido Schimmel; +Cc: netdev, davem, kuba, pabeni, edumazet, dsahern, mlxsw
Hello:
This series was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 14 Sep 2022 10:53:37 +0300 you wrote:
> Patch #1 fixes a bug in ipmr code.
>
> Patch #2 adds corresponding test cases.
>
> Ido Schimmel (2):
> ipmr: Always call ip{,6}_mr_forward() from RCU read-side critical
> section
> selftests: forwarding: Add test cases for unresolved multicast routes
>
> [...]
Here is the summary with links:
- [net,1/2] ipmr: Always call ip{,6}_mr_forward() from RCU read-side critical section
https://git.kernel.org/netdev/net/c/b07a9b26e2b1
- [net,2/2] selftests: forwarding: Add test cases for unresolved multicast routes
https://git.kernel.org/netdev/net/c/2b5a8c8f59d9
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-09-20 15:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-14 7:53 [PATCH net 0/2] ipmr: Always call ip{,6}_mr_forward() from RCU read-side critical section Ido Schimmel
2022-09-14 7:53 ` [PATCH net 1/2] " Ido Schimmel
2022-09-14 7:53 ` [PATCH net 2/2] selftests: forwarding: Add test cases for unresolved multicast routes Ido Schimmel
2022-09-20 15:40 ` [PATCH net 0/2] ipmr: Always call ip{,6}_mr_forward() from RCU read-side critical section patchwork-bot+netdevbpf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.