[PATCH net] ipv4: igmp: report interface group allocation failure on join

[PATCH net] ipv4: igmp: report interface group allocation failure on join

[Tianyi Chen]

setsockopt(IP_ADD_MEMBERSHIP) can return 0 while the group was never
joined on the interface.  __ip_mc_join_group() publishes the socket-side
ip_mc_socklist in inet->mc_list before calling ____ip_mc_inc_group().
That helper returns void, so when its kzalloc_obj() fails it returns
without creating the interface-side ip_mc_list and the syscall still
reports success.  The socket then passes IP_MSFILTER but cannot receive
the group's traffic: no IGMP report is generated, the group is absent
from /proc/net/igmp, and the receive path drops the datagrams.  Closing
the socket also decrements the interface group for every published
ip_mc_socklist, so it can remove a group that another socket joined.

Make the helper return -ENOBUFS on allocation failure, call it before
publishing iml, and free iml on failure.  The object is charged with
sock_kmalloc(), so sock_kfree_s() with the same size keeps
sk_omem_alloc balanced; it was never published and has no source list
yet, so no RCU free is needed.  Successful joins still publish the fully
initialized socket-side record after the interface group exists.  RTNL
serializes the writers, not the RCU readers; this ordering prevents a
published socket membership without a device group.

The legacy void wrapper keeps its best-effort behaviour: interface-up,
init, bridge and tunnel callers cannot report an allocation failure.  A
query does not recreate a missing group; only a later successful
inc_group call can.  The silent failure predates the 2019 helper split
(9fb20801dab4); there is no recent regression to tag with Fixes.

Reproduced with fail-nth on v6.19 and v7.3-rc2.  N=2 fails the helper's
kzalloc_obj(): the original returns 0 with no interface group and 0/4
multicast datagrams delivered; this change returns -ENOBUFS, leaves no
socket or interface state, and a later join delivers 2/2.  On the
original, closing the half-joined socket also removes a second socket's
group; the patched kernel keeps it.  Normal joins, the existing-group
users++ path and source-specific joins (allowed 2/2, filtered 0/2) are
unchanged.

Assisted-by: LLM
Signed-off-by: Tianyi Chen <hi@tychen.cc>
---
 net/ipv4/igmp.c | 37 ++++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index d56355aca797..81a100330630 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -1566,9 +1566,15 @@ static void inet_ifmcaddr_notify(struct net_device *dev,
 
 /*
  *	A socket has joined a multicast group on device dev.
+ *
+ *	Returns 0 if the interface now has a reference to the group, or
+ *	-ENOBUFS if the new group could not be allocated.  Callers that can
+ *	report an error to userspace must check it: a socket that reports a
+ *	successful join while the interface has no group cannot receive the
+ *	group's traffic.
  */
-static void ____ip_mc_inc_group(struct in_device *in_dev, __be32 addr,
-				unsigned int mode, gfp_t gfp)
+static int ____ip_mc_inc_group(struct in_device *in_dev, __be32 addr,
+			       unsigned int mode, gfp_t gfp)
 {
 	struct ip_mc_list __rcu **mc_hash;
 	struct ip_mc_list *im;
@@ -1595,12 +1601,12 @@ static void ____ip_mc_inc_group(struct in_device *in_dev, __be32 addr,
 	if  (im) {
 		WRITE_ONCE(im->users, im->users + 1);
 		ip_mc_add_src(in_dev, &addr, mode, 0, NULL, 0);
-		goto out;
+		return 0;
 	}
 
 	im = kzalloc_obj(*im, gfp);
 	if (!im)
-		goto out;
+		return -ENOBUFS;
 
 	WRITE_ONCE(im->users, 1);
 	im->interface = in_dev;
@@ -1630,13 +1636,18 @@ static void ____ip_mc_inc_group(struct in_device *in_dev, __be32 addr,
 	inet_ifmcaddr_notify(in_dev->dev, im, RTM_NEWMULTICAST);
 	if (!in_dev->dead)
 		ip_rt_multicast_event(in_dev);
-out:
-	return;
+	return 0;
 }
 
 void __ip_mc_inc_group(struct in_device *in_dev, __be32 addr, gfp_t gfp)
 {
-	____ip_mc_inc_group(in_dev, addr, MCAST_EXCLUDE, gfp);
+	/*
+	 * Legacy void interface: interface-up, init, bridge and tunnel
+	 * callers cannot report an allocation failure.  Keep the best-effort
+	 * behaviour.  A query does not recreate a missing group; only a later
+	 * successful inc_group call can.
+	 */
+	(void)____ip_mc_inc_group(in_dev, addr, MCAST_EXCLUDE, gfp);
 }
 EXPORT_SYMBOL(__ip_mc_inc_group);
 
@@ -2386,11 +2397,19 @@ static int __ip_mc_join_group(struct sock *sk, struct ip_mreqn *imr,
 		goto done;
 
 	memcpy(&iml->multi, imr, sizeof(*imr));
-	iml->next_rcu = inet->mc_list;
 	iml->sflist = NULL;
 	iml->sfmode = mode;
+	/*
+	 * Make the interface group exist before publishing the socket's
+	 * membership: success must mean the group can actually receive.
+	 */
+	err = ____ip_mc_inc_group(in_dev, addr, mode, GFP_KERNEL);
+	if (err) {
+		sock_kfree_s(sk, iml, sizeof(*iml));
+		goto done;
+	}
+	iml->next_rcu = inet->mc_list;
 	rcu_assign_pointer(inet->mc_list, iml);
-	____ip_mc_inc_group(in_dev, addr, mode, GFP_KERNEL);
 	err = 0;
 done:
 	return err;

base-commit: df2908090cda368b01ff43709f51890076c56157
-- 
2.56.0

Re: [PATCH net] ipv4: igmp: report interface group allocation failure on join

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.