Netdev List
 help / color / mirror / Atom feed
* [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

* [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 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 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

* 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

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