Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next] ipv4: igmp: annotate data-races around im->users
@ 2026-05-21  9:36 Yuyang Huang
  2026-05-21 10:53 ` Jiayuan Chen
  0 siblings, 1 reply; 3+ messages in thread
From: Yuyang Huang @ 2026-05-21  9:36 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 IPv4 multicast memberships under RCU and
prints im->users without holding RTNL, while multicast join and leave
paths update the field while holding RTNL. Annotate this intentional
lockless snapshot with READ_ONCE() and the matching writers with
WRITE_ONCE().

Signed-off-by: Yuyang Huang <sigefriedhyy@gmail.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 27d120183779..f2aca659b29c 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -1541,7 +1541,7 @@ static void ____ip_mc_inc_group(struct in_device *in_dev, __be32 addr,
 	}
 
 	if  (im) {
-		im->users++;
+		WRITE_ONCE(im->users, im->users + 1);
 		ip_mc_add_src(in_dev, &addr, mode, 0, NULL, 0);
 		goto out;
 	}
@@ -1550,7 +1550,7 @@ static void ____ip_mc_inc_group(struct in_device *in_dev, __be32 addr,
 	if (!im)
 		goto out;
 
-	im->users = 1;
+	WRITE_ONCE(im->users, 1);
 	im->interface = in_dev;
 	in_dev_hold(in_dev);
 	im->multiaddr = addr;
@@ -1784,7 +1784,10 @@ void __ip_mc_dec_group(struct in_device *in_dev, __be32 addr, gfp_t gfp)
 	     (i = rtnl_dereference(*ip)) != NULL;
 	     ip = &i->next_rcu) {
 		if (i->multiaddr == addr) {
-			if (--i->users == 0) {
+			int new_users = i->users - 1;
+
+			WRITE_ONCE(i->users, new_users);
+			if (new_users == 0) {
 				ip_mc_hash_remove(in_dev, i);
 				*ip = i->next_rcu;
 				in_dev->mc_count--;
@@ -2977,7 +2980,7 @@ static int igmp_mc_seq_show(struct seq_file *seq, void *v)
 		delta = im->timer.expires - jiffies;
 		seq_printf(seq,
 			   "\t\t\t\t%08X %5d %d:%08lX\t\t%d\n",
-			   im->multiaddr, im->users,
+			   im->multiaddr, READ_ONCE(im->users),
 			   im->tm_running,
 			   im->tm_running ? jiffies_delta_to_clock_t(delta) : 0,
 			   im->reporter);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next] ipv4: igmp: annotate data-races around im->users
  2026-05-21  9:36 [PATCH net-next] ipv4: igmp: annotate data-races around im->users Yuyang Huang
@ 2026-05-21 10:53 ` Jiayuan Chen
  2026-05-21 11:02   ` Uyo Ko
  0 siblings, 1 reply; 3+ messages in thread
From: Jiayuan Chen @ 2026-05-21 10:53 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


On 5/21/26 5:36 PM, Yuyang Huang wrote:
> /proc/net/igmp walks IPv4 multicast memberships under RCU and
> prints im->users without holding RTNL, while multicast join and leave
> paths update the field while holding RTNL. Annotate this intentional
> lockless snapshot with READ_ONCE() and the matching writers with
> WRITE_ONCE().
>
> Signed-off-by: Yuyang Huang <sigefriedhyy@gmail.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 27d120183779..f2aca659b29c 100644
> --- a/net/ipv4/igmp.c
> +++ b/net/ipv4/igmp.c
> @@ -1541,7 +1541,7 @@ static void ____ip_mc_inc_group(struct in_device *in_dev, __be32 addr,
>   	}
>   
>   	if  (im) {
> -		im->users++;
> +		WRITE_ONCE(im->users, im->users + 1);
>   		ip_mc_add_src(in_dev, &addr, mode, 0, NULL, 0);
>   		goto out;
>   	}
> @@ -1550,7 +1550,7 @@ static void ____ip_mc_inc_group(struct in_device *in_dev, __be32 addr,
>   	if (!im)
>   		goto out;
>   
> -	im->users = 1;
> +	WRITE_ONCE(im->users, 1);
>   	im->interface = in_dev;
>   	in_dev_hold(in_dev);
>   	im->multiaddr = addr;
> @@ -1784,7 +1784,10 @@ void __ip_mc_dec_group(struct in_device *in_dev, __be32 addr, gfp_t gfp)
>   	     (i = rtnl_dereference(*ip)) != NULL;
>   	     ip = &i->next_rcu) {
>   		if (i->multiaddr == addr) {
> -			if (--i->users == 0) {
> +			int new_users = i->users - 1;
> +
> +			WRITE_ONCE(i->users, new_users);
> +			if (new_users == 0) {

Using a local variable here feels slightly awkward, but it does save one 
extra load compared

to re-reading i->users after the WRITE_ONCE(), so it's acceptable.


One suggestion: please add a Fixes: tag, like other data-race fixes 
annotated with READ_ONCE()/WRITE_ONCE().

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next] ipv4: igmp: annotate data-races around im->users
  2026-05-21 10:53 ` Jiayuan Chen
@ 2026-05-21 11:02   ` Uyo Ko
  0 siblings, 0 replies; 3+ messages in thread
From: Uyo Ko @ 2026-05-21 11:02 UTC (permalink / raw)
  To: Jiayuan Chen
  Cc: David S. Miller, David Ahern, Eric Dumazet, Ido Schimmel,
	Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel, netdev

>One suggestion: please add a Fixes: tag, like other data-race fixes
>annotated with READ_ONCE()/WRITE_ONCE().

Thanks for the prompt review, I will adjust it in patch v2.


On Thu, May 21, 2026 at 7:53 PM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
>
> On 5/21/26 5:36 PM, Yuyang Huang wrote:
> > /proc/net/igmp walks IPv4 multicast memberships under RCU and
> > prints im->users without holding RTNL, while multicast join and leave
> > paths update the field while holding RTNL. Annotate this intentional
> > lockless snapshot with READ_ONCE() and the matching writers with
> > WRITE_ONCE().
> >
> > Signed-off-by: Yuyang Huang <sigefriedhyy@gmail.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 27d120183779..f2aca659b29c 100644
> > --- a/net/ipv4/igmp.c
> > +++ b/net/ipv4/igmp.c
> > @@ -1541,7 +1541,7 @@ static void ____ip_mc_inc_group(struct in_device *in_dev, __be32 addr,
> >       }
> >
> >       if  (im) {
> > -             im->users++;
> > +             WRITE_ONCE(im->users, im->users + 1);
> >               ip_mc_add_src(in_dev, &addr, mode, 0, NULL, 0);
> >               goto out;
> >       }
> > @@ -1550,7 +1550,7 @@ static void ____ip_mc_inc_group(struct in_device *in_dev, __be32 addr,
> >       if (!im)
> >               goto out;
> >
> > -     im->users = 1;
> > +     WRITE_ONCE(im->users, 1);
> >       im->interface = in_dev;
> >       in_dev_hold(in_dev);
> >       im->multiaddr = addr;
> > @@ -1784,7 +1784,10 @@ void __ip_mc_dec_group(struct in_device *in_dev, __be32 addr, gfp_t gfp)
> >            (i = rtnl_dereference(*ip)) != NULL;
> >            ip = &i->next_rcu) {
> >               if (i->multiaddr == addr) {
> > -                     if (--i->users == 0) {
> > +                     int new_users = i->users - 1;
> > +
> > +                     WRITE_ONCE(i->users, new_users);
> > +                     if (new_users == 0) {
>
> Using a local variable here feels slightly awkward, but it does save one
> extra load compared
>
> to re-reading i->users after the WRITE_ONCE(), so it's acceptable.
>
>
> One suggestion: please add a Fixes: tag, like other data-race fixes
> annotated with READ_ONCE()/WRITE_ONCE().

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-21 11:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-21  9:36 [PATCH net-next] ipv4: igmp: annotate data-races around im->users Yuyang Huang
2026-05-21 10:53 ` Jiayuan Chen
2026-05-21 11:02   ` Uyo Ko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox