[PATCH net-next 0/3] ipv6: mcast: add data-race annotations

[PATCH net-next 0/3] ipv6: mcast: add data-race annotations

[Eric Dumazet]

ipv6_chk_mcast_addr() and igmp6_mcf_seq_show() are reading
fields under RCU. Add missing annotations.

Eric Dumazet (3):
  ipv6: mcast: reduce ipv6_chk_mcast_addr() indentation
  ipv6: mcast: annotate data-races around mc->mca_sfcount[MCAST_EXCLUDE]
  ipv6: mcast: annotate data-race around psf->sf_count[MCAST_XXX]

 net/ipv6/mcast.c | 59 ++++++++++++++++++++++++++----------------------
 1 file changed, 32 insertions(+), 27 deletions(-)

-- 
2.47.0.338.g60cca15819-goog

[PATCH net-next 1/3] ipv6: mcast: reduce ipv6_chk_mcast_addr() indentation

[Eric Dumazet]

Add a label and two gotos to shorten lines by two tabulations,
to ease code review of following patches.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv6/mcast.c | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index b244dbf61d5f3969c8886a999166ad5ef68b746d..afe707b6841d1ce84f11cc588200615b504a591d 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1021,29 +1021,31 @@ bool ipv6_chk_mcast_addr(struct net_device *dev, const struct in6_addr *group,
 
 	rcu_read_lock();
 	idev = __in6_dev_get(dev);
-	if (idev) {
-		for_each_mc_rcu(idev, mc) {
-			if (ipv6_addr_equal(&mc->mca_addr, group))
-				break;
-		}
-		if (mc) {
-			if (src_addr && !ipv6_addr_any(src_addr)) {
-				struct ip6_sf_list *psf;
+	if (!idev)
+		goto unlock;
+	for_each_mc_rcu(idev, mc) {
+		if (ipv6_addr_equal(&mc->mca_addr, group))
+			break;
+	}
+	if (!mc)
+		goto unlock;
+	if (src_addr && !ipv6_addr_any(src_addr)) {
+		struct ip6_sf_list *psf;
 
-				for_each_psf_rcu(mc, psf) {
-					if (ipv6_addr_equal(&psf->sf_addr, src_addr))
-						break;
-				}
-				if (psf)
-					rv = psf->sf_count[MCAST_INCLUDE] ||
-						psf->sf_count[MCAST_EXCLUDE] !=
-						mc->mca_sfcount[MCAST_EXCLUDE];
-				else
-					rv = mc->mca_sfcount[MCAST_EXCLUDE] != 0;
-			} else
-				rv = true; /* don't filter unspecified source */
+		for_each_psf_rcu(mc, psf) {
+			if (ipv6_addr_equal(&psf->sf_addr, src_addr))
+				break;
 		}
+		if (psf)
+			rv = psf->sf_count[MCAST_INCLUDE] ||
+				psf->sf_count[MCAST_EXCLUDE] !=
+				mc->mca_sfcount[MCAST_EXCLUDE];
+		else
+			rv = mc->mca_sfcount[MCAST_EXCLUDE] != 0;
+	} else {
+		rv = true; /* don't filter unspecified source */
 	}
+unlock:
 	rcu_read_unlock();
 	return rv;
 }
-- 
2.47.0.338.g60cca15819-goog

[PATCH net-next 2/3] ipv6: mcast: annotate data-races around mc->mca_sfcount[MCAST_EXCLUDE]

[Eric Dumazet]

mc->mca_sfcount[MCAST_EXCLUDE] is read locklessly from
ipv6_chk_mcast_addr().

Add READ_ONCE() and WRITE_ONCE() annotations accordingly.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv6/mcast.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index afe707b6841d1ce84f11cc588200615b504a591d..09622142b0705bd81491f148dde4612c0f8fddb8 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1039,9 +1039,9 @@ bool ipv6_chk_mcast_addr(struct net_device *dev, const struct in6_addr *group,
 		if (psf)
 			rv = psf->sf_count[MCAST_INCLUDE] ||
 				psf->sf_count[MCAST_EXCLUDE] !=
-				mc->mca_sfcount[MCAST_EXCLUDE];
+				READ_ONCE(mc->mca_sfcount[MCAST_EXCLUDE]);
 		else
-			rv = mc->mca_sfcount[MCAST_EXCLUDE] != 0;
+			rv = READ_ONCE(mc->mca_sfcount[MCAST_EXCLUDE]) != 0;
 	} else {
 		rv = true; /* don't filter unspecified source */
 	}
@@ -2505,7 +2505,8 @@ static int ip6_mc_add_src(struct inet6_dev *idev, const struct in6_addr *pmca,
 	sf_markstate(pmc);
 	isexclude = pmc->mca_sfmode == MCAST_EXCLUDE;
 	if (!delta)
-		pmc->mca_sfcount[sfmode]++;
+		WRITE_ONCE(pmc->mca_sfcount[sfmode],
+			   pmc->mca_sfcount[sfmode] + 1);
 	err = 0;
 	for (i = 0; i < sfcount; i++) {
 		err = ip6_mc_add1_src(pmc, sfmode, &psfsrc[i]);
@@ -2516,7 +2517,8 @@ static int ip6_mc_add_src(struct inet6_dev *idev, const struct in6_addr *pmca,
 		int j;
 
 		if (!delta)
-			pmc->mca_sfcount[sfmode]--;
+			WRITE_ONCE(pmc->mca_sfcount[sfmode],
+				   pmc->mca_sfcount[sfmode] - 1);
 		for (j = 0; j < i; j++)
 			ip6_mc_del1_src(pmc, sfmode, &psfsrc[j]);
 	} else if (isexclude != (pmc->mca_sfcount[MCAST_EXCLUDE] != 0)) {
@@ -2561,7 +2563,8 @@ static void ip6_mc_clear_src(struct ifmcaddr6 *pmc)
 	RCU_INIT_POINTER(pmc->mca_sources, NULL);
 	pmc->mca_sfmode = MCAST_EXCLUDE;
 	pmc->mca_sfcount[MCAST_INCLUDE] = 0;
-	pmc->mca_sfcount[MCAST_EXCLUDE] = 1;
+	/* Paired with the READ_ONCE() from ipv6_chk_mcast_addr() */
+	WRITE_ONCE(pmc->mca_sfcount[MCAST_EXCLUDE], 1);
 }
 
 /* called with mc_lock */
-- 
2.47.0.338.g60cca15819-goog

[PATCH net-next 3/3] ipv6: mcast: annotate data-race around psf->sf_count[MCAST_XXX]

[Eric Dumazet]

psf->sf_count[MCAST_XXX] fields are read locklessly from
ipv6_chk_mcast_addr() and igmp6_mcf_seq_show().

Add READ_ONCE() and WRITE_ONCE() annotations accordingly.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv6/mcast.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 09622142b0705bd81491f148dde4612c0f8fddb8..5ca8692d565d5055eeebef2a547ced217d81c7d4 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1037,8 +1037,8 @@ bool ipv6_chk_mcast_addr(struct net_device *dev, const struct in6_addr *group,
 				break;
 		}
 		if (psf)
-			rv = psf->sf_count[MCAST_INCLUDE] ||
-				psf->sf_count[MCAST_EXCLUDE] !=
+			rv = READ_ONCE(psf->sf_count[MCAST_INCLUDE]) ||
+				READ_ONCE(psf->sf_count[MCAST_EXCLUDE]) !=
 				READ_ONCE(mc->mca_sfcount[MCAST_EXCLUDE]);
 		else
 			rv = READ_ONCE(mc->mca_sfcount[MCAST_EXCLUDE]) != 0;
@@ -2287,7 +2287,7 @@ static int ip6_mc_del1_src(struct ifmcaddr6 *pmc, int sfmode,
 		/* source filter not found, or count wrong =>  bug */
 		return -ESRCH;
 	}
-	psf->sf_count[sfmode]--;
+	WRITE_ONCE(psf->sf_count[sfmode], psf->sf_count[sfmode] - 1);
 	if (!psf->sf_count[MCAST_INCLUDE] && !psf->sf_count[MCAST_EXCLUDE]) {
 		struct inet6_dev *idev = pmc->idev;
 
@@ -2393,7 +2393,7 @@ static int ip6_mc_add1_src(struct ifmcaddr6 *pmc, int sfmode,
 			rcu_assign_pointer(pmc->mca_sources, psf);
 		}
 	}
-	psf->sf_count[sfmode]++;
+	WRITE_ONCE(psf->sf_count[sfmode], psf->sf_count[sfmode] + 1);
 	return 0;
 }
 
@@ -3079,8 +3079,8 @@ static int igmp6_mcf_seq_show(struct seq_file *seq, void *v)
 			   state->dev->ifindex, state->dev->name,
 			   &state->im->mca_addr,
 			   &psf->sf_addr,
-			   psf->sf_count[MCAST_INCLUDE],
-			   psf->sf_count[MCAST_EXCLUDE]);
+			   READ_ONCE(psf->sf_count[MCAST_INCLUDE]),
+			   READ_ONCE(psf->sf_count[MCAST_EXCLUDE]));
 	}
 	return 0;
 }
-- 
2.47.0.338.g60cca15819-goog

Re: [PATCH net-next 1/3] ipv6: mcast: reduce ipv6_chk_mcast_addr() indentation

[David Ahern]

On 12/10/24 11:33 AM, Eric Dumazet wrote:
> Add a label and two gotos to shorten lines by two tabulations,
> to ease code review of following patches.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>  net/ipv6/mcast.c | 42 ++++++++++++++++++++++--------------------
>  1 file changed, 22 insertions(+), 20 deletions(-)
> 

Reviewed-by: David Ahern <dsahern@kernel.org>

Re: [PATCH net-next 2/3] ipv6: mcast: annotate data-races around mc->mca_sfcount[MCAST_EXCLUDE]

[David Ahern]

On 12/10/24 11:33 AM, Eric Dumazet wrote:
> mc->mca_sfcount[MCAST_EXCLUDE] is read locklessly from
> ipv6_chk_mcast_addr().
> 
> Add READ_ONCE() and WRITE_ONCE() annotations accordingly.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>  net/ipv6/mcast.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 

Reviewed-by: David Ahern <dsahern@kernel.org>

Re: [PATCH net-next 3/3] ipv6: mcast: annotate data-race around psf->sf_count[MCAST_XXX]

[David Ahern]

On 12/10/24 11:33 AM, Eric Dumazet wrote:
> psf->sf_count[MCAST_XXX] fields are read locklessly from
> ipv6_chk_mcast_addr() and igmp6_mcf_seq_show().
> 
> Add READ_ONCE() and WRITE_ONCE() annotations accordingly.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>  net/ipv6/mcast.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 

Reviewed-by: David Ahern <dsahern@kernel.org>

Re: [PATCH net-next 0/3] ipv6: mcast: add data-race annotations

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 10 Dec 2024 18:33:49 +0000 you wrote:
> ipv6_chk_mcast_addr() and igmp6_mcf_seq_show() are reading
> fields under RCU. Add missing annotations.
> 
> Eric Dumazet (3):
>   ipv6: mcast: reduce ipv6_chk_mcast_addr() indentation
>   ipv6: mcast: annotate data-races around mc->mca_sfcount[MCAST_EXCLUDE]
>   ipv6: mcast: annotate data-race around psf->sf_count[MCAST_XXX]
> 
> [...]

Here is the summary with links:
  - [net-next,1/3] ipv6: mcast: reduce ipv6_chk_mcast_addr() indentation
    https://git.kernel.org/netdev/net-next/c/d51cfd5f4fe0
  - [net-next,2/3] ipv6: mcast: annotate data-races around mc->mca_sfcount[MCAST_EXCLUDE]
    https://git.kernel.org/netdev/net-next/c/626962911ad8
  - [net-next,3/3] ipv6: mcast: annotate data-race around psf->sf_count[MCAST_XXX]
    https://git.kernel.org/netdev/net-next/c/00bf2032e976

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html