| Message ID | 20241017174109.85717-6-stefan.wiehler@nokia.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Lock RCU before calling ip6mr_get_table() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net |
| netdev/apply | fail | Patch does not apply to net-0 |
Stefan Wiehler <stefan.wiehler@nokia.com> wrote: > + rtnl_lock(); > mrt = ip6mr_new_table(net, RT6_TABLE_DFLT); > if (IS_ERR(mrt)) { > err = PTR_ERR(mrt); > goto err1; > } > + rtnl_unlock(); > > err = fib_default_rule_add(ops, 0x7fff, RT6_TABLE_DFLT, 0); > if (err < 0) > @@ -254,6 +256,7 @@ static int __net_init ip6mr_rules_init(struct net *net) > ip6mr_free_table(mrt); > rtnl_unlock(); > err1: > + rtnl_unlock(); Looks like a double-unlock?
>> + rtnl_lock(); >> mrt = ip6mr_new_table(net, RT6_TABLE_DFLT); >> if (IS_ERR(mrt)) { >> err = PTR_ERR(mrt); >> goto err1; >> } >> + rtnl_unlock(); >> >> err = fib_default_rule_add(ops, 0x7fff, RT6_TABLE_DFLT, 0); >> if (err < 0) >> @@ -254,6 +256,7 @@ static int __net_init ip6mr_rules_init(struct net *net) >> ip6mr_free_table(mrt); >> rtnl_unlock(); >> err1: >> + rtnl_unlock(); > > Looks like a double-unlock? Thanks, will fix in v7.
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c index 017f9e31edfb..9bf42aafc7f0 100644 --- a/net/ipv6/ip6mr.c +++ b/net/ipv6/ip6mr.c @@ -236,11 +236,13 @@ static int __net_init ip6mr_rules_init(struct net *net) INIT_LIST_HEAD(&net->ipv6.mr6_tables); + rtnl_lock(); mrt = ip6mr_new_table(net, RT6_TABLE_DFLT); if (IS_ERR(mrt)) { err = PTR_ERR(mrt); goto err1; } + rtnl_unlock(); err = fib_default_rule_add(ops, 0x7fff, RT6_TABLE_DFLT, 0); if (err < 0) @@ -254,6 +256,7 @@ static int __net_init ip6mr_rules_init(struct net *net) ip6mr_free_table(mrt); rtnl_unlock(); err1: + rtnl_unlock(); fib_rules_unregister(ops); return err; }
When IPV6_MROUTE_MULTIPLE_TABLES is enabled, multicast routing tables must be read under RCU or RTNL lock. Since mr_table_alloc() can sleep, we call ip6mr_new_table() under the RTNL lock. Detected by Lockdep-RCU: [ 10.247131] WARNING: suspicious RCU usage [ 10.247133] 6.1.103-49518b10de-nokia_sm_x86 #1 Not tainted [ 10.247135] ----------------------------- [ 10.247137] /net/ipv6/ip6mr.c:131 RCU-list traversed in non-reader section!! [ 10.247140] other info that might help us debug this: [ 10.247142] rcu_scheduler_active = 2, debug_locks = 1 [ 10.247144] 1 lock held by swapper/0/1: [ 10.247147] #0: ffffffff82b374d0 (pernet_ops_rwsem){+.+.}-{3:3}, at: register_pernet_subsys+0x15/0x40 [ 10.247164] stack backtrace: [ 10.247166] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.1.103-49518b10de-nokia_sm_x86 #1 [ 10.247170] Hardware name: Nokia Asil/Default string, BIOS 0ACNA114 07/18/2024 [ 10.247175] Call Trace: [ 10.247178] <TASK> [ 10.247181] dump_stack_lvl+0xb7/0xe9 [ 10.247189] lockdep_rcu_suspicious.cold+0x2d/0x64 [ 10.247198] ip6mr_get_table+0x8a/0x90 [ 10.247203] ip6mr_net_init+0x7c/0x200 [ 10.247209] ops_init+0x37/0x1f0 [ 10.247215] register_pernet_operations+0x129/0x230 [ 10.247221] ? af_unix_init+0xca/0xca [ 10.247227] register_pernet_subsys+0x24/0x40 [ 10.247231] ip6_mr_init+0x42/0xf2 [ 10.247235] inet6_init+0x133/0x3b9 [ 10.247238] do_one_initcall+0x74/0x290 [ 10.247247] kernel_init_freeable+0x251/0x294 [ 10.247253] ? rest_init+0x174/0x174 [ 10.247257] kernel_init+0x16/0x12c [ 10.247260] ret_from_fork+0x1f/0x30 [ 10.247271] </TASK> Signed-off-by: Stefan Wiehler <stefan.wiehler@nokia.com> Fixes: d1db275dd3f6 ("ipv6: ip6mr: support multiple tables") --- net/ipv6/ip6mr.c | 3 +++ 1 file changed, 3 insertions(+)