[PATCH net v1] igmp: use copy-on-write RCU updates in ip_mc_source()
flat view
HOTtoday
From: Binbin Deng <hidden>
Date: 2026-11-02 09:53:50
Also in:
lkml
Subsystem:
networking [general], networking [ipv4/ipv6], the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern, Ido Schimmel, Linus Torvalds
pmc->sflist is read locklessly under rcu_read_lock() by ip_mc_sf_allow() during packet reception in the UDP and RAW multicast receive paths. ip_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 the new source was written into the array. The setsockopt writer holds RTNL and the socket lock, but neither excludes ip_mc_sf_allow(), which traverses the filter inside an RCU read-side critical section. During an expansion, the race looks like this: CPU0 CPU1 ip_mc_source(add = true) ip_mc_sf_allow() /* RTNL and socket lock held */ allocate newpsl copy old sources into newpsl newpsl->sl_count = old source count rcu_assign_pointer(pmc->sflist, newpsl) psl = newpsl rcu_read_lock() psl = rcu_dereference(pmc->sflist) read psl->sl_count shift psl->sl_addr[] for insertion psl->sl_addr[i] = source read psl->sl_addr[i] psl->sl_count++ apply source-filter decision rcu_read_unlock() The in-place add and remove paths have the same problem after a reader has obtained the old list: the writer shifts or appends source addresses and updates sl_count while the reader iterates those fields. This is a data race and can expose a mixed old/new filter state, with duplicated or missed source entries. A multicast packet may consequently be accepted or dropped contrary to the configured filter. An ordinary UDP socket can trigger the race: one thread updates a source membership while multicast traffic is received and filtered on another CPU. No special hardware is required. Fix this by allocating and fully populating the replacement filter before publishing it with rcu_assign_pointer(), and by reclaiming the old filter with kfree_rcu(). The socket source list is therefore never modified while an RCU reader may still be traversing it. Fixes: c85bb41e9318 ("igmp: fix ip_mc_sf_allow race [v5]") Signed-off-by: Binbin Deng <redacted> --- net/ipv4/igmp.c | 107 +++++++++++++++++++++++++++++------------------- 1 file changed, 65 insertions(+), 42 deletions(-)
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index d56355aca797..f965091500da 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c@@ -2489,10 +2489,10 @@ int ip_mc_source(int add, int omode, struct sock *sk, struct struct ip_mc_socklist *pmc; struct in_device *in_dev = NULL; struct inet_sock *inet = inet_sk(sk); - struct ip_sf_socklist *psl; + struct ip_sf_socklist *newpsl, *psl; struct net *net = sock_net(sk); int leavegroup = 0; - int i, j, rv; + int count, i, j, rv; if (!ipv4_is_multicast(addr)) return -EINVAL;
@@ -2554,13 +2554,34 @@ int ip_mc_source(int add, int omode, struct sock *sk, struct 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 */ ip_mc_del_src(in_dev, &mreqs->imr_multiaddr, omode, 1, &mreqs->imr_sourceaddr, 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); err = 0; goto done; }
@@ -2570,49 +2591,51 @@ int ip_mc_source(int add, int omode, struct sock *sk, struct err = -ENOBUFS; goto done; } - if (!psl || psl->sl_count == psl->sl_max) { - struct ip_sf_socklist *newpsl; - int count = IP_SFBLOCK; - if (psl) - count += psl->sl_max; - newpsl = sock_kmalloc(sk, struct_size(newpsl, sl_addr, count), - GFP_KERNEL); - if (!newpsl) { - err = -ENOBUFS; - goto done; - } - newpsl->sl_max = count; - newpsl->sl_count = count - IP_SFBLOCK; - if (psl) { - for (i = 0; i < psl->sl_count; i++) - newpsl->sl_addr[i] = psl->sl_addr[i]; - /* decrease mem now to avoid the memleak warning */ - atomic_sub(struct_size(psl, sl_addr, psl->sl_max), - &sk->sk_omem_alloc); + rv = 1; + if (psl) { + for (i = 0; i < psl->sl_count; i++) { + rv = memcmp(&psl->sl_addr[i], &mreqs->imr_sourceaddr, + sizeof(__be32)); + if (rv == 0) + break; } - rcu_assign_pointer(pmc->sflist, newpsl); - if (psl) - kfree_rcu(psl, rcu); - psl = newpsl; - } - rv = 1; /* > 0 for insert logic below if sl_count is 0 */ - for (i = 0; i < psl->sl_count; i++) { - rv = memcmp(&psl->sl_addr[i], &mreqs->imr_sourceaddr, - sizeof(__be32)); - if (rv == 0) - break; - } + if (rv == 0) /* address already there is an error */ goto done; - for (j = psl->sl_count-1; j >= i; j--) - psl->sl_addr[j+1] = psl->sl_addr[j]; - psl->sl_addr[i] = mreqs->imr_sourceaddr; - psl->sl_count++; - err = 0; + + i = psl ? psl->sl_count + 1 : 1; + count = IP_SFBLOCK; + if (psl) + count += psl->sl_max; + newpsl = sock_kmalloc(sk, struct_size(newpsl, sl_addr, count), + GFP_KERNEL); + if (!newpsl) { + err = -ENOBUFS; + goto done; + } + newpsl->sl_max = count; + newpsl->sl_count = i; + if (psl) { + for (j = 0; j < psl->sl_count; j++) + newpsl->sl_addr[j] = psl->sl_addr[j]; + } + newpsl->sl_addr[i - 1] = mreqs->imr_sourceaddr; + /* update the interface list */ ip_mc_add_src(in_dev, &mreqs->imr_multiaddr, omode, 1, &mreqs->imr_sourceaddr, 1); + + if (psl) { + /* decrease mem now to avoid the memleak warning */ + atomic_sub(struct_size(psl, sl_addr, psl->sl_max), + &sk->sk_omem_alloc); + } + rcu_assign_pointer(pmc->sflist, newpsl); + if (psl) + kfree_rcu(psl, rcu); + err = 0; + done: if (leavegroup) err = ip_mc_leave_group(sk, &imr);
--
2.43.0