Re: [PATCH v2 net 2/5] ipv6: mcast: use copy-on-write RCU updates in ip6_mc_source()

[Ido Schimmel <idosch@nvidia.com>]

On Fri, Aug 28, 2026 at 08:45:28AM +0000, Eric Dumazet wrote:
> pmc->sflist is read locklessly under rcu_read_lock() by
> inet6_mc_check() during packet reception in the UDP and RAW
> multicast receive paths.
> 
> ip6_mc_source() mutated psl->sl_addr and psl->sl_count in-place
> when adding or removing a source filter. Additionally, when expanding
> the filter buffer, newpsl was published via rcu_assign_pointer()
> before writing the new source into the array.
> 
> Because 16-byte struct in6_addr writes are not atomic and array
> shifting is not synchronized with RCU readers, concurrent readers in
> inet6_mc_check() could read torn IPv6 addresses or observe
> duplicated/missed source entries.
> 
> Fix this by switching ip6_mc_source() to copy-on-write RCU updates:
> allocate and fully populate newpsl before publishing it via
> rcu_assign_pointer(), and reclaim the old filter via kfree_rcu(),
> matching ip6_mc_msfilter().
> 
> Also remove the now unused IP6_SFBLOCK macro.
> 
> Fixes: 882ba1f73c06 ("mld: convert ipv6_mc_socklist->sflist to RCU")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Taehee Yoo <ap420073@gmail.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

[...]

> @@ -424,58 +422,74 @@ int ip6_mc_source(int add, int omode, struct sock *sk,
>  			goto done;
>  		}
>  
> +		atomic_sub(struct_size(psl, sl_addr, psl->sl_max),
> +			   &sk->sk_omem_alloc);
> +
> +		if (psl->sl_count == 1) {
> +			newpsl = NULL;
> +		} else {
> +			newpsl = sock_kmalloc(sk, struct_size(newpsl, sl_addr,
> +							      psl->sl_count - 1),
> +					      GFP_KERNEL);
> +			if (!newpsl) {
> +				atomic_add(struct_size(psl, sl_addr, psl->sl_max),
> +					   &sk->sk_omem_alloc);
> +				err = -ENOBUFS;
> +				goto done;
> +			}
> +			newpsl->sl_max = psl->sl_count - 1;
> +			newpsl->sl_count = psl->sl_count - 1;
> +			for (j = 0; j < i; j++)
> +				newpsl->sl_addr[j] = psl->sl_addr[j];
> +			for (j = i + 1; j < psl->sl_count; j++)
> +				newpsl->sl_addr[j - 1] = psl->sl_addr[j];
> +		}
> +
>  		/* update the interface filter */
>  		ip6_mc_del_src(idev, group, omode, 1, source, 1);
>  
> -		for (j = i+1; j < psl->sl_count; j++)
> -			psl->sl_addr[j-1] = psl->sl_addr[j];
> -		psl->sl_count--;
> +		rcu_assign_pointer(pmc->sflist, newpsl);
> +		kfree_rcu(psl, rcu);

Earlier we have:

	/* if a source filter was set, must be the same mode as before */
	if (rcu_access_pointer(pmc->sflist)) {
		if (pmc->sfmode != omode) {
			err = -EINVAL;
			goto done;
		}
	} else if (pmc->sfmode != omode) {
		/* allow mode switches for empty-set filters */
		ip6_mc_add_src(idev, group, omode, 0, NULL, 0);
		ip6_mc_del_src(idev, group, pmc->sfmode, 0, NULL, 0);
		pmc->sfmode = omode;
	}

So setting the source list to NULL when deleting the last source in
exclude mode looks like a behavior change, but in a good way.

  IPV6_ADD_MEMBERSHIP(G)          sfmode = MCAST_EXCLUDE
  MCAST_BLOCK_SOURCE(G, S)        sflist = { S }
  MCAST_UNBLOCK_SOURCE(G, S)      was { }, now NULL
  MCAST_JOIN_SOURCE_GROUP(G, S2)  was -EINVAL, now OK