* [PATCH net v4 1/5] ip6mr: Lock RCU before ip6mr_get_table() call in ip6mr_vif_seq_start()
2024-10-11 7:23 [PATCH net v4 0/5] Lock RCU before calling ip6mr_get_table() Stefan Wiehler
@ 2024-10-11 7:23 ` Stefan Wiehler
2024-10-11 7:23 ` [PATCH net v4 2/5] ip6mr: Lock RCU before ip6mr_get_table() call in ip6mr_ioctl() Stefan Wiehler
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Stefan Wiehler @ 2024-10-11 7:23 UTC (permalink / raw)
To: David S . Miller, David Ahern, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: netdev, linux-kernel, Stefan Wiehler
When IPV6_MROUTE_MULTIPLE_TABLES is enabled, calls to ip6mr_get_table()
must be done under RCU or RTNL lock.
In case ip6mr_vif_seq_start() fails with an error (currently not
possible, as RT6_TABLE_DFLT always exists, ip6mr_get_table() cannot fail
in this case), ip6mr_vif_seq_stop() would be called and release the RCU
lock.
Signed-off-by: Stefan Wiehler <stefan.wiehler@nokia.com>
Fixes: d1db275dd3f6 ("ipv6: ip6mr: support multiple tables")
Reviewed-by: Eric Dumazet <edumazet@google.com>
---
net/ipv6/ip6mr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index 2ce4ae0d8dc3..268e77196753 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -411,13 +411,13 @@ static void *ip6mr_vif_seq_start(struct seq_file *seq, loff_t *pos)
struct net *net = seq_file_net(seq);
struct mr_table *mrt;
+ rcu_read_lock();
mrt = ip6mr_get_table(net, RT6_TABLE_DFLT);
if (!mrt)
return ERR_PTR(-ENOENT);
iter->mrt = mrt;
- rcu_read_lock();
return mr_vif_seq_start(seq, pos);
}
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net v4 2/5] ip6mr: Lock RCU before ip6mr_get_table() call in ip6mr_ioctl()
2024-10-11 7:23 [PATCH net v4 0/5] Lock RCU before calling ip6mr_get_table() Stefan Wiehler
2024-10-11 7:23 ` [PATCH net v4 1/5] ip6mr: Lock RCU before ip6mr_get_table() call in ip6mr_vif_seq_start() Stefan Wiehler
@ 2024-10-11 7:23 ` Stefan Wiehler
2024-10-11 7:23 ` [PATCH net v4 3/5] ip6mr: Lock RCU before ip6mr_get_table() call in ip6mr_compat_ioctl() Stefan Wiehler
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Stefan Wiehler @ 2024-10-11 7:23 UTC (permalink / raw)
To: David S . Miller, David Ahern, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: netdev, linux-kernel, Stefan Wiehler
When IPV6_MROUTE_MULTIPLE_TABLES is enabled, calls to
ip6mr_get_table() must be done under RCU or RTNL lock.
Detected by Lockdep-RCU:
[ 48.834645] WARNING: suspicious RCU usage
[ 48.834647] 6.1.103-584209f6d5-nokia_sm_x86 #1 Tainted: G S O
[ 48.834649] -----------------------------
[ 48.834651] /net/ipv6/ip6mr.c:132 RCU-list traversed in non-reader section!!
[ 48.834654]
other info that might help us debug this:
[ 48.834656]
rcu_scheduler_active = 2, debug_locks = 1
[ 48.834658] no locks held by radvd/5777.
[ 48.834660]
stack backtrace:
[ 48.834663] CPU: 0 PID: 5777 Comm: radvd Tainted: G S O 6.1.103-584209f6d5-nokia_sm_x86 #1
[ 48.834666] Hardware name: Nokia Asil/Default string, BIOS 0ACNA113 06/07/2024
[ 48.834673] Call Trace:
[ 48.834674] <TASK>
[ 48.834677] dump_stack_lvl+0xb7/0xe9
[ 48.834687] lockdep_rcu_suspicious.cold+0x2d/0x64
[ 48.834697] ip6mr_get_table+0x9f/0xb0
[ 48.834704] ip6mr_ioctl+0x50/0x360
[ 48.834713] ? sk_ioctl+0x5f/0x1c0
[ 48.834719] sk_ioctl+0x5f/0x1c0
[ 48.834723] ? find_held_lock+0x2b/0x80
[ 48.834731] sock_do_ioctl+0x7b/0x140
[ 48.834737] ? proc_nr_files+0x30/0x30
[ 48.834744] sock_ioctl+0x1f5/0x360
[ 48.834754] __x64_sys_ioctl+0x8d/0xd0
[ 48.834760] do_syscall_64+0x3c/0x90
[ 48.834765] entry_SYSCALL_64_after_hwframe+0x6e/0xd8
[ 48.834769] RIP: 0033:0x7fda5f56e2ac [ 48.834773] Code: 1e fa 48 8d 44 24 08 48 89 54 24 e0 48 89 44 24 c0 48 8d 44 24 d0 48 89 44 24 c8 b8 1 0 00 00 00 c7 44 24 b8 10 00 00 00 0f 05 <3d> 00 f0 ff ff 89 c2 77 0b 89 d0 c3 0f 1f 84
00 00 00 00 00 48 8b
[ 48.834776] RSP: 002b:00007fff52d4bda8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 48.834782] RAX: ffffffffffffffda RBX: 000000000171cd80 RCX: 00007fda5f56e2ac
[ 48.834784] RDX: 00007fff52d4bdb0 RSI: 0000000000008913 RDI: 0000000000000003
[ 48.834787] RBP: 000000000171cd30 R08: 0000000000000007 R09: 000000000000003c
[ 48.834789] R10: 0000000000000004 R11: 0000000000000246 R12: 0000000000000003
[ 48.834791] R13: 0000000000000000 R14: 0000000000000004 R15: 000000000040d43c
[ 48.834802] </TASK>
Signed-off-by: Stefan Wiehler <stefan.wiehler@nokia.com>
Fixes: d1db275dd3f6 ("ipv6: ip6mr: support multiple tables")
---
net/ipv6/ip6mr.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index 268e77196753..b18eb4ad21e4 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -1884,18 +1884,23 @@ int ip6mr_ioctl(struct sock *sk, int cmd, void *arg)
struct mfc6_cache *c;
struct net *net = sock_net(sk);
struct mr_table *mrt;
+ int err;
+ rcu_read_lock();
mrt = ip6mr_get_table(net, raw6_sk(sk)->ip6mr_table ? : RT6_TABLE_DFLT);
- if (!mrt)
- return -ENOENT;
+ if (!mrt) {
+ err = -ENOENT;
+ goto out;
+ }
switch (cmd) {
case SIOCGETMIFCNT_IN6:
vr = (struct sioc_mif_req6 *)arg;
- if (vr->mifi >= mrt->maxvif)
- return -EINVAL;
+ if (vr->mifi >= mrt->maxvif) {
+ err = -EINVAL;
+ goto out;
+ }
vr->mifi = array_index_nospec(vr->mifi, mrt->maxvif);
- rcu_read_lock();
vif = &mrt->vif_table[vr->mifi];
if (VIF_EXISTS(mrt, vr->mifi)) {
vr->icount = READ_ONCE(vif->pkt_in);
@@ -1905,12 +1910,11 @@ int ip6mr_ioctl(struct sock *sk, int cmd, void *arg)
rcu_read_unlock();
return 0;
}
- rcu_read_unlock();
- return -EADDRNOTAVAIL;
+ err = -EADDRNOTAVAIL;
+ goto out;
case SIOCGETSGCNT_IN6:
sr = (struct sioc_sg_req6 *)arg;
- rcu_read_lock();
c = ip6mr_cache_find(mrt, &sr->src.sin6_addr,
&sr->grp.sin6_addr);
if (c) {
@@ -1920,11 +1924,16 @@ int ip6mr_ioctl(struct sock *sk, int cmd, void *arg)
rcu_read_unlock();
return 0;
}
- rcu_read_unlock();
- return -EADDRNOTAVAIL;
+ err = -EADDRNOTAVAIL;
+ goto out;
default:
- return -ENOIOCTLCMD;
+ err = -ENOIOCTLCMD;
+ goto out;
}
+
+out:
+ rcu_read_unlock();
+ return err;
}
#ifdef CONFIG_COMPAT
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net v4 3/5] ip6mr: Lock RCU before ip6mr_get_table() call in ip6mr_compat_ioctl()
2024-10-11 7:23 [PATCH net v4 0/5] Lock RCU before calling ip6mr_get_table() Stefan Wiehler
2024-10-11 7:23 ` [PATCH net v4 1/5] ip6mr: Lock RCU before ip6mr_get_table() call in ip6mr_vif_seq_start() Stefan Wiehler
2024-10-11 7:23 ` [PATCH net v4 2/5] ip6mr: Lock RCU before ip6mr_get_table() call in ip6mr_ioctl() Stefan Wiehler
@ 2024-10-11 7:23 ` Stefan Wiehler
2024-10-11 16:04 ` Jakub Kicinski
2024-10-11 7:23 ` [PATCH net v4 4/5] ip6mr: Lock RCU before ip6mr_get_table() call in ip6mr_get_route() Stefan Wiehler
` (2 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Stefan Wiehler @ 2024-10-11 7:23 UTC (permalink / raw)
To: David S . Miller, David Ahern, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: netdev, linux-kernel, Stefan Wiehler
When IPV6_MROUTE_MULTIPLE_TABLES is enabled, calls to ip6mr_get_table()
must be done under RCU or RTNL lock. Copy from user space must be
performed beforehand as we are not allowed to sleep under RCU lock.
Signed-off-by: Stefan Wiehler <stefan.wiehler@nokia.com>
Fixes: d1db275dd3f6 ("ipv6: ip6mr: support multiple tables")
---
net/ipv6/ip6mr.c | 48 ++++++++++++++++++++++++++++++++----------------
1 file changed, 32 insertions(+), 16 deletions(-)
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index b18eb4ad21e4..1e233ee15d43 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -1961,19 +1961,36 @@ int ip6mr_compat_ioctl(struct sock *sk, unsigned int cmd, void __user *arg)
struct mfc6_cache *c;
struct net *net = sock_net(sk);
struct mr_table *mrt;
-
- mrt = ip6mr_get_table(net, raw6_sk(sk)->ip6mr_table ? : RT6_TABLE_DFLT);
- if (!mrt)
- return -ENOENT;
+ int err;
switch (cmd) {
case SIOCGETMIFCNT_IN6:
if (copy_from_user(&vr, arg, sizeof(vr)))
return -EFAULT;
- if (vr.mifi >= mrt->maxvif)
- return -EINVAL;
+ break;
+ case SIOCGETSGCNT_IN6:
+ if (copy_from_user(&sr, arg, sizeof(sr)))
+ return -EFAULT;
+ break;
+ default:
+ return -ENOIOCTLCMD;
+ }
+
+
+ rcu_read_lock();
+ mrt = ip6mr_get_table(net, raw6_sk(sk)->ip6mr_table ? : RT6_TABLE_DFLT);
+ if (!mrt) {
+ err = -ENOENT;
+ goto out;
+ }
+
+ switch (cmd) {
+ case SIOCGETMIFCNT_IN6:
+ if (vr.mifi >= mrt->maxvif) {
+ err = -EINVAL;
+ goto out;
+ }
vr.mifi = array_index_nospec(vr.mifi, mrt->maxvif);
- rcu_read_lock();
vif = &mrt->vif_table[vr.mifi];
if (VIF_EXISTS(mrt, vr.mifi)) {
vr.icount = READ_ONCE(vif->pkt_in);
@@ -1987,12 +2004,9 @@ int ip6mr_compat_ioctl(struct sock *sk, unsigned int cmd, void __user *arg)
return 0;
}
rcu_read_unlock();
- return -EADDRNOTAVAIL;
+ err = -EADDRNOTAVAIL;
+ goto out;
case SIOCGETSGCNT_IN6:
- if (copy_from_user(&sr, arg, sizeof(sr)))
- return -EFAULT;
-
- rcu_read_lock();
c = ip6mr_cache_find(mrt, &sr.src.sin6_addr, &sr.grp.sin6_addr);
if (c) {
sr.pktcnt = c->_c.mfc_un.res.pkt;
@@ -2004,11 +2018,13 @@ int ip6mr_compat_ioctl(struct sock *sk, unsigned int cmd, void __user *arg)
return -EFAULT;
return 0;
}
- rcu_read_unlock();
- return -EADDRNOTAVAIL;
- default:
- return -ENOIOCTLCMD;
+ err = -EADDRNOTAVAIL;
+ goto out;
}
+
+out:
+ rcu_read_unlock();
+ return err;
}
#endif
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net v4 4/5] ip6mr: Lock RCU before ip6mr_get_table() call in ip6mr_get_route()
2024-10-11 7:23 [PATCH net v4 0/5] Lock RCU before calling ip6mr_get_table() Stefan Wiehler
` (2 preceding siblings ...)
2024-10-11 7:23 ` [PATCH net v4 3/5] ip6mr: Lock RCU before ip6mr_get_table() call in ip6mr_compat_ioctl() Stefan Wiehler
@ 2024-10-11 7:23 ` Stefan Wiehler
2024-10-11 7:23 ` [PATCH net v4 5/5] Revert "ipv6: Fix suspicious RCU usage warning in ip6mr" Stefan Wiehler
2024-10-11 11:13 ` [PATCH net v4 0/5] Lock RCU before calling ip6mr_get_table() Eric Dumazet
5 siblings, 0 replies; 9+ messages in thread
From: Stefan Wiehler @ 2024-10-11 7:23 UTC (permalink / raw)
To: David S . Miller, David Ahern, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: netdev, linux-kernel, Stefan Wiehler
When IPV6_MROUTE_MULTIPLE_TABLES is enabled, calls to ip6mr_get_table()
must be done under RCU or RTNL lock.
Signed-off-by: Stefan Wiehler <stefan.wiehler@nokia.com>
Fixes: d1db275dd3f6 ("ipv6: ip6mr: support multiple tables")
---
net/ipv6/ip6mr.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index 1e233ee15d43..a817b688473a 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -2300,11 +2300,13 @@ int ip6mr_get_route(struct net *net, struct sk_buff *skb, struct rtmsg *rtm,
struct mfc6_cache *cache;
struct rt6_info *rt = dst_rt6_info(skb_dst(skb));
+ rcu_read_lock();
mrt = ip6mr_get_table(net, RT6_TABLE_DFLT);
- if (!mrt)
- return -ENOENT;
+ if (!mrt) {
+ err = -ENOENT;
+ goto out;
+ }
- rcu_read_lock();
cache = ip6mr_cache_find(mrt, &rt->rt6i_src.addr, &rt->rt6i_dst.addr);
if (!cache && skb->dev) {
int vif = ip6mr_find_vif(mrt, skb->dev);
@@ -2322,15 +2324,15 @@ int ip6mr_get_route(struct net *net, struct sk_buff *skb, struct rtmsg *rtm,
dev = skb->dev;
if (!dev || (vif = ip6mr_find_vif(mrt, dev)) < 0) {
- rcu_read_unlock();
- return -ENODEV;
+ err = -ENODEV;
+ goto out;
}
/* really correct? */
skb2 = alloc_skb(sizeof(struct ipv6hdr), GFP_ATOMIC);
if (!skb2) {
- rcu_read_unlock();
- return -ENOMEM;
+ err = -ENOMEM;
+ goto out;
}
NETLINK_CB(skb2).portid = portid;
@@ -2352,12 +2354,13 @@ int ip6mr_get_route(struct net *net, struct sk_buff *skb, struct rtmsg *rtm,
iph->daddr = rt->rt6i_dst.addr;
err = ip6mr_cache_unresolved(mrt, vif, skb2, dev);
- rcu_read_unlock();
- return err;
+ goto out;
}
err = mr_fill_mroute(mrt, skb, &cache->_c, rtm);
+
+out:
rcu_read_unlock();
return err;
}
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net v4 5/5] Revert "ipv6: Fix suspicious RCU usage warning in ip6mr"
2024-10-11 7:23 [PATCH net v4 0/5] Lock RCU before calling ip6mr_get_table() Stefan Wiehler
` (3 preceding siblings ...)
2024-10-11 7:23 ` [PATCH net v4 4/5] ip6mr: Lock RCU before ip6mr_get_table() call in ip6mr_get_route() Stefan Wiehler
@ 2024-10-11 7:23 ` Stefan Wiehler
2024-10-11 11:13 ` [PATCH net v4 0/5] Lock RCU before calling ip6mr_get_table() Eric Dumazet
5 siblings, 0 replies; 9+ messages in thread
From: Stefan Wiehler @ 2024-10-11 7:23 UTC (permalink / raw)
To: David S . Miller, David Ahern, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: netdev, linux-kernel, Stefan Wiehler
This reverts commit b6dd5acde3f165e364881c36de942c5b252e2a27.
We should not suppress Lockdep-RCU splats when calling ip6mr_get_table()
without RCU or RTNL lock.
Signed-off-by: Stefan Wiehler <stefan.wiehler@nokia.com>
---
net/ipv6/ip6mr.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index a817b688473a..44e2b4d1ca23 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -105,8 +105,7 @@ static void ipmr_expire_process(struct timer_list *t);
#ifdef CONFIG_IPV6_MROUTE_MULTIPLE_TABLES
#define ip6mr_for_each_table(mrt, net) \
list_for_each_entry_rcu(mrt, &net->ipv6.mr6_tables, list, \
- lockdep_rtnl_is_held() || \
- list_empty(&net->ipv6.mr6_tables))
+ lockdep_rtnl_is_held())
static struct mr_table *ip6mr_mr_table_iter(struct net *net,
struct mr_table *mrt)
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH net v4 0/5] Lock RCU before calling ip6mr_get_table()
2024-10-11 7:23 [PATCH net v4 0/5] Lock RCU before calling ip6mr_get_table() Stefan Wiehler
` (4 preceding siblings ...)
2024-10-11 7:23 ` [PATCH net v4 5/5] Revert "ipv6: Fix suspicious RCU usage warning in ip6mr" Stefan Wiehler
@ 2024-10-11 11:13 ` Eric Dumazet
2024-10-14 15:04 ` Stefan Wiehler
5 siblings, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2024-10-11 11:13 UTC (permalink / raw)
To: Stefan Wiehler
Cc: David S . Miller, David Ahern, Jakub Kicinski, Paolo Abeni,
netdev, linux-kernel
On Fri, Oct 11, 2024 at 9:48 AM Stefan Wiehler <stefan.wiehler@nokia.com> wrote:
>
> Lock RCU before calling ip6mr_get_table() in several ip6mr functions.
>
> v4:
> - mention in commit message that ip6mr_vif_seq_stop() would be called
> in case ip6mr_vif_seq_start() returns an error
> - fix unitialised use of mrt variable
> - revert commit b6dd5acde3f1 ("ipv6: Fix suspicious RCU usage warning
> in ip6mr")
Hi Stefan. I think a v5 is needed :)
Please double check your syslog
[ 18.149447] =============================
[ 18.149471] WARNING: suspicious RCU usage
[ 18.149649] 6.12.0-rc2-virtme #1155 Not tainted
[ 18.149726] -----------------------------
[ 18.149747] net/ipv6/ip6mr.c:131 RCU-list traversed in non-reader section!!
[ 18.149792]
other info that might help us debug this:
[ 18.149824]
rcu_scheduler_active = 2, debug_locks = 1
[ 18.150050] 1 lock held by swapper/0/1:
[ 18.150090] #0: ffffffff95b36390 (pernet_ops_rwsem){+.+.}-{3:3},
at: register_pernet_subsys (net/core/net_namespace.c:1356)
[ 18.151482]
stack backtrace:
[ 18.151716] CPU: 12 UID: 0 PID: 1 Comm: swapper/0 Not tainted
6.12.0-rc2-virtme #1155
[ 18.151809] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 18.151982] Call Trace:
[ 18.152122] <TASK>
[ 18.152411] dump_stack_lvl (lib/dump_stack.c:123)
[ 18.152411] lockdep_rcu_suspicious (kernel/locking/lockdep.c:6822)
[ 18.152411] ip6mr_get_table (net/ipv6/ip6mr.c:131 (discriminator 9))
[ 18.152411] ip6mr_net_init (net/ipv6/ip6mr.c:384
net/ipv6/ip6mr.c:238 net/ipv6/ip6mr.c:1317 net/ipv6/ip6mr.c:1309)
[ 18.152411] ops_init (net/core/net_namespace.c:139)
[ 18.152411] register_pernet_operations
(net/core/net_namespace.c:1239 net/core/net_namespace.c:1315)
[ 18.152411] register_pernet_subsys (net/core/net_namespace.c:1357)
[ 18.152411] ip6_mr_init (net/ipv6/ip6mr.c:1379)
[ 18.152411] inet6_init (net/ipv6/af_inet6.c:1137)
[ 18.152411] ? __pfx_inet6_init (net/ipv6/af_inet6.c:1076)
[ 18.152411] do_one_initcall (init/main.c:1269)
[ 18.152411] ? _raw_spin_unlock_irqrestore
(./arch/x86/include/asm/irqflags.h:42
./arch/x86/include/asm/irqflags.h:97
./arch/x86/include/asm/irqflags.h:155
./include/linux/spinlock_api_smp.h:151 kernel/locking/spinlock.c:194)
[ 18.152411] kernel_init_freeable (init/main.c:1330 (discriminator
1) init/main.c:1347 (discriminator 1) init/main.c:1366 (discriminator
1) init/main.c:1580 (discriminator 1))
[ 18.152411] ? __pfx_kernel_init (init/main.c:1461)
[ 18.152411] kernel_init (init/main.c:1471)
[ 18.152411] ret_from_fork (arch/x86/kernel/process.c:153)
[ 18.152411] ? __pfx_kernel_init (init/main.c:1461)
[ 18.152411] ret_from_fork_asm (arch/x86/entry/entry_64.S:257)
[ 18.152411] </TASK>
Thank you.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net v4 0/5] Lock RCU before calling ip6mr_get_table()
2024-10-11 11:13 ` [PATCH net v4 0/5] Lock RCU before calling ip6mr_get_table() Eric Dumazet
@ 2024-10-14 15:04 ` Stefan Wiehler
0 siblings, 0 replies; 9+ messages in thread
From: Stefan Wiehler @ 2024-10-14 15:04 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, David Ahern, Jakub Kicinski, Paolo Abeni,
netdev, linux-kernel
> Hi Stefan. I think a v5 is needed :)
>
> Please double check your syslog
>
> [ 18.149447] =============================
> [ 18.149471] WARNING: suspicious RCU usage
> [ 18.149649] 6.12.0-rc2-virtme #1155 Not tainted
> [ 18.149726] -----------------------------
> [ 18.149747] net/ipv6/ip6mr.c:131 RCU-list traversed in non-reader section!!
> [ 18.149792]
> other info that might help us debug this:
>
> [ 18.149824]
> rcu_scheduler_active = 2, debug_locks = 1
> [ 18.150050] 1 lock held by swapper/0/1:
> [ 18.150090] #0: ffffffff95b36390 (pernet_ops_rwsem){+.+.}-{3:3},
> at: register_pernet_subsys (net/core/net_namespace.c:1356)
> [ 18.151482]
> stack backtrace:
> [ 18.151716] CPU: 12 UID: 0 PID: 1 Comm: swapper/0 Not tainted
> 6.12.0-rc2-virtme #1155
> [ 18.151809] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> [ 18.151982] Call Trace:
> [ 18.152122] <TASK>
> [ 18.152411] dump_stack_lvl (lib/dump_stack.c:123)
> [ 18.152411] lockdep_rcu_suspicious (kernel/locking/lockdep.c:6822)
> [ 18.152411] ip6mr_get_table (net/ipv6/ip6mr.c:131 (discriminator 9))
> [ 18.152411] ip6mr_net_init (net/ipv6/ip6mr.c:384
> net/ipv6/ip6mr.c:238 net/ipv6/ip6mr.c:1317 net/ipv6/ip6mr.c:1309)
> [ 18.152411] ops_init (net/core/net_namespace.c:139)
> [ 18.152411] register_pernet_operations
> (net/core/net_namespace.c:1239 net/core/net_namespace.c:1315)
> [ 18.152411] register_pernet_subsys (net/core/net_namespace.c:1357)
> [ 18.152411] ip6_mr_init (net/ipv6/ip6mr.c:1379)
> [ 18.152411] inet6_init (net/ipv6/af_inet6.c:1137)
> [ 18.152411] ? __pfx_inet6_init (net/ipv6/af_inet6.c:1076)
> [ 18.152411] do_one_initcall (init/main.c:1269)
> [ 18.152411] ? _raw_spin_unlock_irqrestore
> (./arch/x86/include/asm/irqflags.h:42
> ./arch/x86/include/asm/irqflags.h:97
> ./arch/x86/include/asm/irqflags.h:155
> ./include/linux/spinlock_api_smp.h:151 kernel/locking/spinlock.c:194)
> [ 18.152411] kernel_init_freeable (init/main.c:1330 (discriminator
> 1) init/main.c:1347 (discriminator 1) init/main.c:1366 (discriminator
> 1) init/main.c:1580 (discriminator 1))
> [ 18.152411] ? __pfx_kernel_init (init/main.c:1461)
> [ 18.152411] kernel_init (init/main.c:1471)
> [ 18.152411] ret_from_fork (arch/x86/kernel/process.c:153)
> [ 18.152411] ? __pfx_kernel_init (init/main.c:1461)
> [ 18.152411] ret_from_fork_asm (arch/x86/entry/entry_64.S:257)
> [ 18.152411] </TASK>
Thanks, I'm not sure why I missed that one since it also shows up on our v6.1-based kernel.
I went through all the remaining calls of ip6mr_get_table() in ip6mr.c (since
I've picked this topic up from a colleague):
- call in ip6mr_rule_action is safe because fib_rules_lookup() holds RCU lock
- call in ipmr_mfc_seq_start() needs to be in RCU read-side critical section as well
- calls in ip6mr_rtm_(set|get)sockopt() need to be in RCU read-side critical section as well
- call in ip6mr_rtm_getroute() needs to hold RCU read lock earlier as well
- call in ip6mr_rtm_dumproute() is safe because rtnl_register_internal() holds the RTNL lock
I'll prepare a v5; please carefully review as I'm not familar with the codebase...
^ permalink raw reply [flat|nested] 9+ messages in thread