| Message ID | 23bd87600f046ce1f1c93513b6ac8f8152b22bf4.1732270911.git.pabeni@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: fix mcast RCU splats | expand |
On 11/22/24 12:02, Paolo Abeni wrote: > diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c > index c58dd78509a2..c6ad01dc8310 100644 > --- a/net/ipv4/ipmr.c > +++ b/net/ipv4/ipmr.c > @@ -358,6 +358,11 @@ bool ipmr_rule_default(const struct fib_rule *rule) > EXPORT_SYMBOL(ipmr_rule_default); > #endif > > +static bool ipmr_can_free_table(struct net *net) > +{ > + return !check_net(net) || !net->ipv4.mr_rules_ops; Not so good. mr_rules_ops exists only for CONFIG_IP_MROUTE_MULTIPLE_TABLES builds. /P
Hi Paolo, kernel test robot noticed the following build errors: [auto build test ERROR on net/main] url: https://github.com/intel-lab-lkp/linux/commits/Paolo-Abeni/ipmr-add-debug-check-for-mr-table-cleanup/20241125-104108 base: net/main patch link: https://lore.kernel.org/r/23bd87600f046ce1f1c93513b6ac8f8152b22bf4.1732270911.git.pabeni%40redhat.com patch subject: [PATCH net 1/3] ipmr: add debug check for mr table cleanup config: sh-se7712_defconfig (https://download.01.org/0day-ci/archive/20241125/202411251504.ZKzltGDp-lkp@intel.com/config) compiler: sh4-linux-gcc (GCC) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241125/202411251504.ZKzltGDp-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202411251504.ZKzltGDp-lkp@intel.com/ All errors (new ones prefixed by >>): net/ipv4/ipmr.c: In function 'ipmr_can_free_table': >> net/ipv4/ipmr.c:363:46: error: 'struct netns_ipv4' has no member named 'mr_rules_ops'; did you mean 'rules_ops'? 363 | return !check_net(net) || !net->ipv4.mr_rules_ops; | ^~~~~~~~~~~~ | rules_ops vim +363 net/ipv4/ipmr.c 360 361 static bool ipmr_can_free_table(struct net *net) 362 { > 363 return !check_net(net) || !net->ipv4.mr_rules_ops; 364 } 365
Hi Paolo, kernel test robot noticed the following build errors: [auto build test ERROR on net/main] url: https://github.com/intel-lab-lkp/linux/commits/Paolo-Abeni/ipmr-add-debug-check-for-mr-table-cleanup/20241125-104108 base: net/main patch link: https://lore.kernel.org/r/23bd87600f046ce1f1c93513b6ac8f8152b22bf4.1732270911.git.pabeni%40redhat.com patch subject: [PATCH net 1/3] ipmr: add debug check for mr table cleanup config: mips-ip22_defconfig (https://download.01.org/0day-ci/archive/20241125/202411251722.Mg6UtrLH-lkp@intel.com/config) compiler: mips-linux-gcc (GCC) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241125/202411251722.Mg6UtrLH-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202411251722.Mg6UtrLH-lkp@intel.com/ All errors (new ones prefixed by >>): net/ipv6/ip6mr.c: In function 'ip6mr_can_free_table': >> net/ipv6/ip6mr.c:346:46: error: 'struct netns_ipv6' has no member named 'mr6_rules_ops'; did you mean 'fib6_rules_ops'? 346 | return !check_net(net) || !net->ipv6.mr6_rules_ops; | ^~~~~~~~~~~~~ | fib6_rules_ops vim +346 net/ipv6/ip6mr.c 343 344 static bool ip6mr_can_free_table(struct net *net) 345 { > 346 return !check_net(net) || !net->ipv6.mr6_rules_ops; 347 } 348
Hi Paolo,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net/main]
url: https://github.com/intel-lab-lkp/linux/commits/Paolo-Abeni/ipmr-add-debug-check-for-mr-table-cleanup/20241125-104108
base: net/main
patch link: https://lore.kernel.org/r/23bd87600f046ce1f1c93513b6ac8f8152b22bf4.1732270911.git.pabeni%40redhat.com
patch subject: [PATCH net 1/3] ipmr: add debug check for mr table cleanup
config: arc-randconfig-001-20241126 (https://download.01.org/0day-ci/archive/20241126/202411260343.02pIupsk-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241126/202411260343.02pIupsk-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411260343.02pIupsk-lkp@intel.com/
All warnings (new ones prefixed by >>):
net/ipv4/ipmr.c: In function 'ipmr_can_free_table':
net/ipv4/ipmr.c:363:46: error: 'struct netns_ipv4' has no member named 'mr_rules_ops'; did you mean 'rules_ops'?
363 | return !check_net(net) || !net->ipv4.mr_rules_ops;
| ^~~~~~~~~~~~
| rules_ops
>> net/ipv4/ipmr.c:364:1: warning: control reaches end of non-void function [-Wreturn-type]
364 | }
| ^
vim +364 net/ipv4/ipmr.c
360
361 static bool ipmr_can_free_table(struct net *net)
362 {
> 363 return !check_net(net) || !net->ipv4.mr_rules_ops;
> 364 }
365
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c index c58dd78509a2..c6ad01dc8310 100644 --- a/net/ipv4/ipmr.c +++ b/net/ipv4/ipmr.c @@ -358,6 +358,11 @@ bool ipmr_rule_default(const struct fib_rule *rule) EXPORT_SYMBOL(ipmr_rule_default); #endif +static bool ipmr_can_free_table(struct net *net) +{ + return !check_net(net) || !net->ipv4.mr_rules_ops; +} + static inline int ipmr_hash_cmp(struct rhashtable_compare_arg *arg, const void *ptr) { @@ -413,6 +418,10 @@ static struct mr_table *ipmr_new_table(struct net *net, u32 id) static void ipmr_free_table(struct mr_table *mrt) { + struct net *net = read_pnet(&mrt->net); + + DEBUG_NET_WARN_ON_ONCE(!ipmr_can_free_table(net)); + timer_shutdown_sync(&mrt->ipmr_expire_timer); mroute_clean_tables(mrt, MRT_FLUSH_VIFS | MRT_FLUSH_VIFS_STATIC | MRT_FLUSH_MFC | MRT_FLUSH_MFC_STATIC); diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c index d66f58932a79..275784a8f35b 100644 --- a/net/ipv6/ip6mr.c +++ b/net/ipv6/ip6mr.c @@ -341,6 +341,11 @@ static unsigned int ip6mr_rules_seq_read(const struct net *net) } #endif +static bool ip6mr_can_free_table(struct net *net) +{ + return !check_net(net) || !net->ipv6.mr6_rules_ops; +} + static int ip6mr_hash_cmp(struct rhashtable_compare_arg *arg, const void *ptr) { @@ -392,6 +397,10 @@ static struct mr_table *ip6mr_new_table(struct net *net, u32 id) static void ip6mr_free_table(struct mr_table *mrt) { + struct net *net = read_pnet(&mrt->net); + + DEBUG_NET_WARN_ON_ONCE(!ip6mr_can_free_table(net)); + timer_shutdown_sync(&mrt->ipmr_expire_timer); mroute_clean_tables(mrt, MRT6_FLUSH_MIFS | MRT6_FLUSH_MIFS_STATIC | MRT6_FLUSH_MFC | MRT6_FLUSH_MFC_STATIC);
The multicast route tables lifecycle, for both ipv4 and ipv6, is protected by RCU using the RTNL lock for write access. In many places a table pointer escapes the RCU (or RTNL) protected critical section, but such scenarios are actually safe because tables are deleted only at namespace cleanup time or just after allocation, in case of default rule creation failure. Tables freed at namespace cleanup time are assured to be alive for the whole netns lifetime; tables freed just after creation time are never exposed to other possible users. Ensure that the free conditions are respected in ip{,6}mr_free_table, to document the locking schema and to prevent future possible introduction of 'table del' operation from breaking it. Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- net/ipv4/ipmr.c | 9 +++++++++ net/ipv6/ip6mr.c | 9 +++++++++ 2 files changed, 18 insertions(+)