* [PATCH net-next 0/2] ipv4: igmp: annotate diagnostic procfs data races
@ 2026-05-31 3:07 Yuyang Huang
2026-05-31 3:07 ` [PATCH 1/2] ipv4: igmp: annotate data-races around in_dev->mc_count Yuyang Huang
2026-05-31 3:07 ` [PATCH 2/2] ipv4: igmp: annotate data-races around timer-related fields Yuyang Huang
0 siblings, 2 replies; 10+ messages in thread
From: Yuyang Huang @ 2026-05-31 3:07 UTC (permalink / raw)
To: Yuyang Huang
Cc: David S. Miller, David Ahern, Eric Dumazet, Ido Schimmel,
Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel, netdev
This patch series addresses several unannotated data races between lockless
RCU-protected diagnostic reads in /proc/net/igmp (igmp_mc_seq_show())
and concurrent writes in serialized paths (RTNL and group spinlocks).
Following the precedent in commit 061c0aa740d5 ("ipv4: igmp: annotate
data-races around im->users"), we annotate these intentional data races
using READ_ONCE() and WRITE_ONCE() macros.
- Patch 1 annotates races around `in_dev->mc_count` (interface-level joins).
- Patch 2 annotates races around active timer-related state tracking fields
(`tm_running`, `reporter`, `expires`) on individual multicast groups.
Yuyang Huang (2):
ipv4: igmp: annotate data-races around in_dev->mc_count
ipv4: igmp: annotate data-races around timer-related fields
net/ipv4/igmp.c | 35 ++++++++++++++++++++---------------
1 file changed, 20 insertions(+), 15 deletions(-)
--
2.54.0.823.g6e5bcc1fc9-goog
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/2] ipv4: igmp: annotate data-races around in_dev->mc_count 2026-05-31 3:07 [PATCH net-next 0/2] ipv4: igmp: annotate diagnostic procfs data races Yuyang Huang @ 2026-05-31 3:07 ` Yuyang Huang 2026-06-04 9:11 ` Paolo Abeni 2026-06-04 10:19 ` Ido Schimmel 2026-05-31 3:07 ` [PATCH 2/2] ipv4: igmp: annotate data-races around timer-related fields Yuyang Huang 1 sibling, 2 replies; 10+ messages in thread From: Yuyang Huang @ 2026-05-31 3:07 UTC (permalink / raw) To: Yuyang Huang Cc: David S. Miller, David Ahern, Eric Dumazet, Ido Schimmel, Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel, netdev /proc/net/igmp walks the multicast list for IPv4 interfaces locklessly under RCU and prints state->in_dev->mc_count. Concurrently, device init/destruction and multicast join/leave paths update the count under the RTNL lock. Fix this intentional lockless snapshot by annotating the read with READ_ONCE() and the updates with WRITE_ONCE(). Fixes: 1d7138de878d ("igmp: RCU conversion of in_dev->mc_list") Signed-off-by: Yuyang Huang <yuyanghuang@google.com> --- net/ipv4/igmp.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c index f2aca659b29c..fd0faf042fa6 100644 --- a/net/ipv4/igmp.c +++ b/net/ipv4/igmp.c @@ -1566,7 +1566,7 @@ static void ____ip_mc_inc_group(struct in_device *in_dev, __be32 addr, #endif im->next_rcu = in_dev->mc_list; - in_dev->mc_count++; + WRITE_ONCE(in_dev->mc_count, in_dev->mc_count + 1); rcu_assign_pointer(in_dev->mc_list, im); ip_mc_hash_add(in_dev, im); @@ -1790,7 +1790,8 @@ void __ip_mc_dec_group(struct in_device *in_dev, __be32 addr, gfp_t gfp) if (new_users == 0) { ip_mc_hash_remove(in_dev, i); *ip = i->next_rcu; - in_dev->mc_count--; + WRITE_ONCE(in_dev->mc_count, + in_dev->mc_count - 1); __igmp_group_dropped(i, gfp); inet_ifmcaddr_notify(in_dev->dev, i, RTM_DELMULTICAST); @@ -1922,7 +1923,7 @@ void ip_mc_destroy_dev(struct in_device *in_dev) while ((i = rtnl_dereference(in_dev->mc_list)) != NULL) { in_dev->mc_list = i->next_rcu; - in_dev->mc_count--; + WRITE_ONCE(in_dev->mc_count, in_dev->mc_count - 1); ip_mc_clear_src(i); ip_ma_put(i); } @@ -2974,7 +2975,9 @@ static int igmp_mc_seq_show(struct seq_file *seq, void *v) if (rcu_access_pointer(state->in_dev->mc_list) == im) { seq_printf(seq, "%d\t%-10s: %5d %7s\n", - state->dev->ifindex, state->dev->name, state->in_dev->mc_count, querier); + state->dev->ifindex, state->dev->name, + READ_ONCE(state->in_dev->mc_count), + querier); } delta = im->timer.expires - jiffies; -- 2.54.0.823.g6e5bcc1fc9-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] ipv4: igmp: annotate data-races around in_dev->mc_count 2026-05-31 3:07 ` [PATCH 1/2] ipv4: igmp: annotate data-races around in_dev->mc_count Yuyang Huang @ 2026-06-04 9:11 ` Paolo Abeni 2026-06-04 9:15 ` Paolo Abeni 2026-06-04 10:19 ` Ido Schimmel 1 sibling, 1 reply; 10+ messages in thread From: Paolo Abeni @ 2026-06-04 9:11 UTC (permalink / raw) To: Yuyang Huang Cc: David S. Miller, David Ahern, Eric Dumazet, Ido Schimmel, Jakub Kicinski, Simon Horman, linux-kernel, netdev On 5/31/26 5:07 AM, Yuyang Huang wrote: > @@ -1922,7 +1923,7 @@ void ip_mc_destroy_dev(struct in_device *in_dev) > > while ((i = rtnl_dereference(in_dev->mc_list)) != NULL) { > in_dev->mc_list = i->next_rcu; > - in_dev->mc_count--; > + WRITE_ONCE(in_dev->mc_count, in_dev->mc_count - 1); > ip_mc_clear_src(i); > ip_ma_put(i); The patch LGTM, but note that sashiko has identified a pre-existing issue which could deserve a follow-up: https://sashiko.dev/#/patchset/20260531030705.3754389-1-yuyanghuang%40google.com /P > } > @@ -2974,7 +2975,9 @@ static int igmp_mc_seq_show(struct seq_file *seq, void *v) > > if (rcu_access_pointer(state->in_dev->mc_list) == im) { > seq_printf(seq, "%d\t%-10s: %5d %7s\n", > - state->dev->ifindex, state->dev->name, state->in_dev->mc_count, querier); > + state->dev->ifindex, state->dev->name, > + READ_ONCE(state->in_dev->mc_count), > + querier); > } > > delta = im->timer.expires - jiffies; ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] ipv4: igmp: annotate data-races around in_dev->mc_count 2026-06-04 9:11 ` Paolo Abeni @ 2026-06-04 9:15 ` Paolo Abeni 2026-06-04 10:17 ` Ido Schimmel 0 siblings, 1 reply; 10+ messages in thread From: Paolo Abeni @ 2026-06-04 9:15 UTC (permalink / raw) To: Yuyang Huang Cc: David S. Miller, David Ahern, Eric Dumazet, Ido Schimmel, Jakub Kicinski, Simon Horman, linux-kernel, netdev On 6/4/26 11:11 AM, Paolo Abeni wrote: > On 5/31/26 5:07 AM, Yuyang Huang wrote: >> @@ -1922,7 +1923,7 @@ void ip_mc_destroy_dev(struct in_device *in_dev) >> >> while ((i = rtnl_dereference(in_dev->mc_list)) != NULL) { >> in_dev->mc_list = i->next_rcu; >> - in_dev->mc_count--; >> + WRITE_ONCE(in_dev->mc_count, in_dev->mc_count - 1); >> ip_mc_clear_src(i); >> ip_ma_put(i); > > The patch LGTM, But it does not apply cleanly to net. Please rebase and report, adding the target tree (`net`) and a revision number (`v2`) in the subj prefix. /P ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] ipv4: igmp: annotate data-races around in_dev->mc_count 2026-06-04 9:15 ` Paolo Abeni @ 2026-06-04 10:17 ` Ido Schimmel 0 siblings, 0 replies; 10+ messages in thread From: Ido Schimmel @ 2026-06-04 10:17 UTC (permalink / raw) To: Paolo Abeni Cc: Yuyang Huang, David S. Miller, David Ahern, Eric Dumazet, Jakub Kicinski, Simon Horman, linux-kernel, netdev On Thu, Jun 04, 2026 at 11:15:29AM +0200, Paolo Abeni wrote: > On 6/4/26 11:11 AM, Paolo Abeni wrote: > > On 5/31/26 5:07 AM, Yuyang Huang wrote: > >> @@ -1922,7 +1923,7 @@ void ip_mc_destroy_dev(struct in_device *in_dev) > >> > >> while ((i = rtnl_dereference(in_dev->mc_list)) != NULL) { > >> in_dev->mc_list = i->next_rcu; > >> - in_dev->mc_count--; > >> + WRITE_ONCE(in_dev->mc_count, in_dev->mc_count - 1); > >> ip_mc_clear_src(i); > >> ip_ma_put(i); > > > > The patch LGTM, > But it does not apply cleanly to net. Please rebase and report, adding > the target tree (`net`) and a revision number (`v2`) in the subj prefix. Paolo, I think it's better to target such patches at net-next and dropping the fixes tag. See for example this recent patch that went into net-next: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/net/ipv4/igmp.c?id=061c0aa740d5d3847cd600a74c66a165bee1fbe0 And this message from Linus: https://lwn.net/Articles/1074171/ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] ipv4: igmp: annotate data-races around in_dev->mc_count 2026-05-31 3:07 ` [PATCH 1/2] ipv4: igmp: annotate data-races around in_dev->mc_count Yuyang Huang 2026-06-04 9:11 ` Paolo Abeni @ 2026-06-04 10:19 ` Ido Schimmel 2026-06-04 11:41 ` Yuyang Huang 1 sibling, 1 reply; 10+ messages in thread From: Ido Schimmel @ 2026-06-04 10:19 UTC (permalink / raw) To: Yuyang Huang Cc: David S. Miller, David Ahern, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel, netdev On Sun, May 31, 2026 at 11:07:03AM +0800, Yuyang Huang wrote: > /proc/net/igmp walks the multicast list for IPv4 interfaces locklessly > under RCU and prints state->in_dev->mc_count. Concurrently, device > init/destruction and multicast join/leave paths update the count > under the RTNL lock. Fix this intentional lockless snapshot by > annotating the read with READ_ONCE() and the updates with WRITE_ONCE(). > > Fixes: 1d7138de878d ("igmp: RCU conversion of in_dev->mc_list") Pending Paolo's approval, please drop the Fixes tag given the patch is targeted at net-next. > Signed-off-by: Yuyang Huang <yuyanghuang@google.com> Code looks fine: Reviewed-by: Ido Schimmel <idosch@nvidia.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] ipv4: igmp: annotate data-races around in_dev->mc_count 2026-06-04 10:19 ` Ido Schimmel @ 2026-06-04 11:41 ` Yuyang Huang 0 siblings, 0 replies; 10+ messages in thread From: Yuyang Huang @ 2026-06-04 11:41 UTC (permalink / raw) To: Ido Schimmel Cc: David S. Miller, David Ahern, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel, netdev >The patch LGTM, but note that sashiko has identified a pre-existing issue which could deserve a follow-up: >https://sashiko.dev/#/patchset/20260531030705.3754389-1-yuyanghuang%40google.com Acked, will create a follow up change to fix this. > > But it does not apply cleanly to net. Please rebase and report, adding > > the target tree (`net`) and a revision number (`v2`) in the subj prefix. > > Paolo, I think it's better to target such patches at net-next and > dropping the fixes tag. > > See for example this recent patch that went into net-next: > > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/net/ipv4/igmp.c?id=061c0aa740d5d3847cd600a74c66a165bee1fbe0 > > And this message from Linus: > > https://lwn.net/Articles/1074171/ > > Pending Paolo's approval, please drop the Fixes tag given the patch is > targeted at net-next. > Acked, will drop the fixed tag in patch series v2. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] ipv4: igmp: annotate data-races around timer-related fields 2026-05-31 3:07 [PATCH net-next 0/2] ipv4: igmp: annotate diagnostic procfs data races Yuyang Huang 2026-05-31 3:07 ` [PATCH 1/2] ipv4: igmp: annotate data-races around in_dev->mc_count Yuyang Huang @ 2026-05-31 3:07 ` Yuyang Huang 2026-06-04 10:20 ` Ido Schimmel 1 sibling, 1 reply; 10+ messages in thread From: Yuyang Huang @ 2026-05-31 3:07 UTC (permalink / raw) To: Yuyang Huang Cc: David S. Miller, David Ahern, Eric Dumazet, Ido Schimmel, Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel, netdev /proc/net/igmp walks the multicast list locklessly under RCU and reads timer-related fields (im->tm_running, im->reporter, im->timer.expires) to print the timer state of multicast memberships. Concurrently, these fields are modified under im->lock spinlock in timer management paths (igmp_stop_timer(), igmp_start_timer(), and igmp_timer_expire()). Fix this intentional lockless snapshot by annotating the lockless reads with READ_ONCE() and the updates with WRITE_ONCE(). Fixes: 1d7138de878d ("igmp: RCU conversion of in_dev->mc_list") Signed-off-by: Yuyang Huang <yuyanghuang@google.com> --- net/ipv4/igmp.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c index fd0faf042fa6..1e958027068b 100644 --- a/net/ipv4/igmp.c +++ b/net/ipv4/igmp.c @@ -220,8 +220,8 @@ static void igmp_stop_timer(struct ip_mc_list *im) spin_lock_bh(&im->lock); if (timer_delete(&im->timer)) refcount_dec(&im->refcnt); - im->tm_running = 0; - im->reporter = 0; + WRITE_ONCE(im->tm_running, 0); + WRITE_ONCE(im->reporter, 0); im->unsolicit_count = 0; spin_unlock_bh(&im->lock); } @@ -231,7 +231,7 @@ static void igmp_start_timer(struct ip_mc_list *im, int max_delay) { int tv = get_random_u32_below(max_delay); - im->tm_running = 1; + WRITE_ONCE(im->tm_running, 1); if (refcount_inc_not_zero(&im->refcnt)) { if (mod_timer(&im->timer, jiffies + tv + 2)) ip_ma_put(im); @@ -267,7 +267,7 @@ static void igmp_mod_timer(struct ip_mc_list *im, int max_delay) if (timer_delete(&im->timer)) { if ((long)(im->timer.expires-jiffies) < max_delay) { add_timer(&im->timer); - im->tm_running = 1; + WRITE_ONCE(im->tm_running, 1); spin_unlock_bh(&im->lock); return; } @@ -857,12 +857,12 @@ static void igmp_timer_expire(struct timer_list *t) struct in_device *in_dev = im->interface; spin_lock(&im->lock); - im->tm_running = 0; + WRITE_ONCE(im->tm_running, 0); if (im->unsolicit_count && --im->unsolicit_count) igmp_start_timer(im, unsolicited_report_interval(in_dev)); - im->reporter = 1; + WRITE_ONCE(im->reporter, 1); spin_unlock(&im->lock); if (IGMP_V1_SEEN(in_dev)) @@ -1325,7 +1325,7 @@ static void __igmp_group_dropped(struct ip_mc_list *im, gfp_t gfp) !READ_ONCE(net->ipv4.sysctl_igmp_llm_reports)) return; - reporter = im->reporter; + reporter = READ_ONCE(im->reporter); igmp_stop_timer(im); if (!in_dev->dead) { @@ -2964,6 +2964,7 @@ static int igmp_mc_seq_show(struct seq_file *seq, void *v) struct igmp_mc_iter_state *state = igmp_mc_seq_private(seq); char *querier; long delta; + int tm_running; #ifdef CONFIG_IP_MULTICAST querier = IGMP_V1_SEEN(state->in_dev) ? "V1" : @@ -2980,13 +2981,14 @@ static int igmp_mc_seq_show(struct seq_file *seq, void *v) querier); } - delta = im->timer.expires - jiffies; + tm_running = READ_ONCE(im->tm_running); + delta = READ_ONCE(im->timer.expires) - jiffies; seq_printf(seq, "\t\t\t\t%08X %5d %d:%08lX\t\t%d\n", im->multiaddr, READ_ONCE(im->users), - im->tm_running, - im->tm_running ? jiffies_delta_to_clock_t(delta) : 0, - im->reporter); + tm_running, + tm_running ? jiffies_delta_to_clock_t(delta) : 0, + READ_ONCE(im->reporter)); } return 0; } -- 2.54.0.823.g6e5bcc1fc9-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] ipv4: igmp: annotate data-races around timer-related fields 2026-05-31 3:07 ` [PATCH 2/2] ipv4: igmp: annotate data-races around timer-related fields Yuyang Huang @ 2026-06-04 10:20 ` Ido Schimmel 2026-06-04 11:35 ` Yuyang Huang 0 siblings, 1 reply; 10+ messages in thread From: Ido Schimmel @ 2026-06-04 10:20 UTC (permalink / raw) To: Yuyang Huang Cc: David S. Miller, David Ahern, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel, netdev On Sun, May 31, 2026 at 11:07:04AM +0800, Yuyang Huang wrote: > /proc/net/igmp walks the multicast list locklessly under RCU and reads > timer-related fields (im->tm_running, im->reporter, im->timer.expires) > to print the timer state of multicast memberships. Concurrently, these > fields are modified under im->lock spinlock in timer management paths > (igmp_stop_timer(), igmp_start_timer(), and igmp_timer_expire()). Fix this > intentional lockless snapshot by annotating the lockless reads with > READ_ONCE() and the updates with WRITE_ONCE(). > > Fixes: 1d7138de878d ("igmp: RCU conversion of in_dev->mc_list") Pending Paolo's approval, please drop the Fixes tag given the patch is targeted at net-next. > Signed-off-by: Yuyang Huang <yuyanghuang@google.com> Reviewed-by: Ido Schimmel <idosch@nvidia.com> [...] > @@ -2964,6 +2964,7 @@ static int igmp_mc_seq_show(struct seq_file *seq, void *v) > struct igmp_mc_iter_state *state = igmp_mc_seq_private(seq); > char *querier; > long delta; > + int tm_running; Nit: Please move this above 'delta' for reverse xmas tree ordering > > #ifdef CONFIG_IP_MULTICAST > querier = IGMP_V1_SEEN(state->in_dev) ? "V1" : ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] ipv4: igmp: annotate data-races around timer-related fields 2026-06-04 10:20 ` Ido Schimmel @ 2026-06-04 11:35 ` Yuyang Huang 0 siblings, 0 replies; 10+ messages in thread From: Yuyang Huang @ 2026-06-04 11:35 UTC (permalink / raw) To: Ido Schimmel Cc: David S. Miller, David Ahern, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel, netdev On Thu, Jun 4, 2026 at 7:20 PM Ido Schimmel <idosch@nvidia.com> wrote: > > > Fixes: 1d7138de878d ("igmp: RCU conversion of in_dev->mc_list") > > Pending Paolo's approval, please drop the Fixes tag given the patch is > targeted at net-next. Thanks, will fix it in patch v2. > > [...] > > > @@ -2964,6 +2964,7 @@ static int igmp_mc_seq_show(struct seq_file *seq, void *v) > > struct igmp_mc_iter_state *state = igmp_mc_seq_private(seq); > > char *querier; > > long delta; > > + int tm_running; > > Nit: Please move this above 'delta' for reverse xmas tree ordering Thanks, will fix it in patch v2. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-06-04 11:42 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-31 3:07 [PATCH net-next 0/2] ipv4: igmp: annotate diagnostic procfs data races Yuyang Huang 2026-05-31 3:07 ` [PATCH 1/2] ipv4: igmp: annotate data-races around in_dev->mc_count Yuyang Huang 2026-06-04 9:11 ` Paolo Abeni 2026-06-04 9:15 ` Paolo Abeni 2026-06-04 10:17 ` Ido Schimmel 2026-06-04 10:19 ` Ido Schimmel 2026-06-04 11:41 ` Yuyang Huang 2026-05-31 3:07 ` [PATCH 2/2] ipv4: igmp: annotate data-races around timer-related fields Yuyang Huang 2026-06-04 10:20 ` Ido Schimmel 2026-06-04 11:35 ` Yuyang Huang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox