mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Junyoung Jang <graypanda.inzag@gmail.com>
To: David Ahern <dsahern@kernel.org>,
	Ido Schimmel <idosch@nvidia.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@kernel.org>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>
Cc: Junyoung Jang <graypanda.inzag@gmail.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH net] igmp: use the joined device when leaving a multicast group
Date: Mon,  9 Nov 2026 21:53:11 +0900	[thread overview]
Message-ID: <20261109125312.2208874-1-graypanda.inzag@gmail.com> (raw)

ip_mc_leave_group() resolves in_dev from the leave request before
matching the request against the socket's multicast membership list.
This lookup can return NULL even while the interface recorded in the
matched membership (iml->multi.imr_ifindex) is still alive:

1. An application joins a group using a local IPv4 address, that
   secondary address is later removed while the interface stays up,
   and the application leaves using the same 8-byte ip_mreq.
2. An unprivileged user joins a group with a 12-byte ip_mreqn
   specifying a valid imr_ifindex and a nonlocal imr_address, then
   leaves with an 8-byte ip_mreq containing that nonlocal address
   and no ifindex.

Because imr_address is nonzero, the early -ENODEV check is bypassed.
The loop then removes and frees the socket's ip_mc_socklist entry
(iml) and returns 0, but skips ip_mc_dec_group() because in_dev is
NULL. Closing the socket cannot release the group via
ip_mc_drop_socket() because the socket membership entry has already
been unlinked. This leaks the interface's struct ip_mc_list reference
and multicast MAC filter beyond the socket's lifetime, allowing an
unprivileged user to repeatedly leak multicast group entries on a
live interface.

Fix this by looking up in_dev from the matched membership's saved
imr_ifindex under RTNL, matching ip_mc_drop_socket(). If the interface
was genuinely removed, inetdev_by_index() still returns NULL and the
existing hot-unplug cleanup behavior is preserved.

Fixes: 4eba7bb1d72d ("ipv4: igmp: Allow removing groups from a removed interface")
Signed-off-by: Junyoung Jang <graypanda.inzag@gmail.com>
---
Tested on Linux 7.4-rc1 in QEMU/KVM as an unprivileged user (UID 1000)
for both the unprivileged ifindex/nonlocal-address mismatch and the
secondary address removal scenario.

Unpatched 7.4-rc1 guest output:
  IGMP_VM_BEGIN kernel=7.4.0-rc1 expect=--expect-leak
  IGMP_VM_CASE=nonlocal-address
  uid=1000
  control_explicit_ifindex
  after_drop=0
  after_close=0
  case_address_only
  join=0
  drop=0
  after_drop=1
  after_close=1
  joins_ok=16
  drops_ok=16
  closes_ok=16
  groups_after=16
  result=PASS
  IGMP_VM_CASE=removed-secondary-address
  uid=1000
  address_removed=1
  primary_present=1
  after_remove=1
  drop=0
  after_drop=1
  after_close=1
  result=PASS

Patched 7.4-rc1 guest output:
  IGMP_VM_BEGIN kernel=7.4.0-rc1 expect=--expect-fixed
  IGMP_VM_CASE=nonlocal-address
  uid=1000
  control_explicit_ifindex
  after_drop=0
  after_close=0
  case_address_only
  join=0
  drop=0
  after_drop=0
  after_close=0
  joins_ok=16
  drops_ok=16
  closes_ok=16
  groups_after=0
  result=PASS
  IGMP_VM_CASE=removed-secondary-address
  uid=1000
  address_removed=1
  primary_present=1
  after_remove=1
  drop=0
  after_drop=0
  after_close=0
  result=PASS

Reproducer (igmp-multicast-reference-leak.c):

/* SPDX-License-Identifier: GPL-2.0-only */
#define _GNU_SOURCE
#include <arpa/inet.h>
#include <errno.h>
#include <ifaddrs.h>
#include <net/if.h>
#include <netinet/in.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <unistd.h>

/*
 * Run in a disposable network namespace. The affected kernel retains
 * groups after a successful address-only leave and socket close.
 */
#define BATCH_GROUPS 16

_Static_assert(sizeof(struct ip_mreqn) == 12, "ip_mreqn must include ifindex");
_Static_assert(sizeof(struct ip_mreq) == 8, "ip_mreq must omit ifindex");

struct observation {
	int before;
	int after_join;
	int after_drop;
	int after_close;
};

struct accumulation {
	int before;
	int after;
	int joins;
	int drops;
	int closes;
};

struct interface_state {
	bool up;
	bool primary_present;
	bool additional_present;
};

static void die(const char *operation)
{
	fprintf(stderr, "%s: %s\n", operation, strerror(errno));
	exit(EXIT_FAILURE);
}

static struct in_addr address(const char *text)
{
	struct in_addr addr;

	if (inet_pton(AF_INET, text, &addr) != 1) {
		fprintf(stderr, "invalid IPv4 address: %s\n", text);
		exit(EXIT_FAILURE);
	}
	return addr;
}

static struct interface_state churn_interface_state(void)
{
	struct in_addr primary = address("192.0.2.1");
	struct in_addr additional = address("192.0.2.10");
	struct interface_state state = { 0 };
	struct ifaddrs *interfaces, *ifa;

	if (getifaddrs(&interfaces))
		die("getifaddrs");
	for (ifa = interfaces; ifa; ifa = ifa->ifa_next) {
		struct sockaddr_in *in;

		/* IPv4 aliases may be reported as eth0:1 by getifaddrs(). */
		if (strncmp(ifa->ifa_name, "eth0", 4) ||
		    (ifa->ifa_name[4] && ifa->ifa_name[4] != ':'))
			continue;
		state.up |= !!(ifa->ifa_flags & IFF_UP);
		if (!ifa->ifa_addr || ifa->ifa_addr->sa_family != AF_INET)
			continue;
		in = (struct sockaddr_in *)ifa->ifa_addr;
		state.primary_present |= in->sin_addr.s_addr == primary.s_addr;
		state.additional_present |= in->sin_addr.s_addr == additional.s_addr;
	}
	freeifaddrs(interfaces);
	return state;
}

/* /proc/net/igmp prints each group as a hexadecimal __be32 value. */
static int group_present(unsigned int ifindex, struct in_addr group)
{
	char line[256], device[IFNAMSIZ];
	unsigned int index, group_count, raw_group, users;
	bool on_interface = false;
	int found = 0;
	FILE *file;

	file = fopen("/proc/net/igmp", "r");
	if (!file)
		die("fopen /proc/net/igmp");

	while (fgets(line, sizeof(line), file)) {
		if (sscanf(line, " %u %15s : %u", &index, device,
			   &group_count) == 3) {
			on_interface = index == ifindex;
			continue;
		}
		if (on_interface &&
		    sscanf(line, " %x %u", &raw_group, &users) == 2 &&
		    raw_group == group.s_addr) {
			found = 1;
			break;
		}
	}
	if (ferror(file))
		die("read /proc/net/igmp");
	if (fclose(file))
		die("fclose /proc/net/igmp");
	return found;
}

static struct ip_mreqn join_request(struct in_addr group,
				    struct in_addr interface_addr, int ifindex)
{
	struct ip_mreqn request = {
		.imr_multiaddr = group,
		.imr_address = interface_addr,
		.imr_ifindex = ifindex,
	};

	return request;
}

static struct observation probe(const char *name, struct in_addr group,
				struct in_addr interface_addr,
				unsigned int ifindex, bool drop_with_ifindex)
{
	struct ip_mreqn join = join_request(group, interface_addr, ifindex);
	struct ip_mreq drop = {
		.imr_multiaddr = group,
		.imr_interface = interface_addr,
	};
	struct observation result;
	int fd;

	printf("%s\n", name);
	result.before = group_present(ifindex, group);
	printf("before=%d\n", result.before);
	fd = socket(AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
	if (fd < 0)
		die("socket");
	printf("socket=%d\n", fd);
	if (setsockopt(fd, IPPROTO_IP, IP_ADD_MEMBERSHIP,
		       &join, sizeof(join)))
		die("IP_ADD_MEMBERSHIP");
	printf("join=0\n");
	result.after_join = group_present(ifindex, group);
	printf("after_join=%d\n", result.after_join);

	if (drop_with_ifindex) {
		if (setsockopt(fd, IPPROTO_IP, IP_DROP_MEMBERSHIP,
			       &join, sizeof(join)))
			die("IP_DROP_MEMBERSHIP (ip_mreqn)");
	} else if (setsockopt(fd, IPPROTO_IP, IP_DROP_MEMBERSHIP,
			      &drop, sizeof(drop))) {
		die("IP_DROP_MEMBERSHIP (ip_mreq)");
	}
	printf("drop=0\n");
	result.after_drop = group_present(ifindex, group);
	printf("after_drop=%d\n", result.after_drop);
	if (close(fd))
		die("close");
	printf("close=0\n");
	result.after_close = group_present(ifindex, group);
	printf("after_close=%d\n", result.after_close);
	return result;
}

static int count_batch_groups(unsigned int ifindex)
{
	char text[INET_ADDRSTRLEN];
	int count = 0;

	for (int i = 1; i <= BATCH_GROUPS; i++) {
		snprintf(text, sizeof(text), "239.250.34.%d", i);
		count += group_present(ifindex, address(text));
	}
	return count;
}

static struct accumulation accumulate_groups(struct in_addr interface_addr,
					     unsigned int ifindex)
{
	char text[INET_ADDRSTRLEN];
	struct accumulation result = { 0 };
	struct ip_mreqn join;
	struct ip_mreq drop;
	int fd;

	result.before = count_batch_groups(ifindex);
	printf("groups_before=%d\n", result.before);
	for (int i = 1; i <= BATCH_GROUPS; i++) {
		snprintf(text, sizeof(text), "239.250.34.%d", i);
		join = join_request(address(text), interface_addr, ifindex);
		drop.imr_multiaddr = join.imr_multiaddr;
		drop.imr_interface = interface_addr;
		fd = socket(AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
		if (fd < 0)
			die("batch socket");
		if (setsockopt(fd, IPPROTO_IP, IP_ADD_MEMBERSHIP,
			       &join, sizeof(join)))
			die("batch IP_ADD_MEMBERSHIP");
		result.joins++;
		if (setsockopt(fd, IPPROTO_IP, IP_DROP_MEMBERSHIP,
			       &drop, sizeof(drop)))
			die("batch IP_DROP_MEMBERSHIP");
		result.drops++;
		if (close(fd))
			die("batch close");
		result.closes++;
	}
	result.after = count_batch_groups(ifindex);
	printf("joins_ok=%d\ndrops_ok=%d\ncloses_ok=%d\ngroups_after=%d\n",
	       result.joins, result.drops, result.closes, result.after);
	return result;
}

static int run_original(bool expect_leak, unsigned int ifindex)
{
	struct in_addr interface_addr = address("203.0.113.77");
	struct observation control, affected;
	struct accumulation batch;
	bool pass;

	control = probe("control_explicit_ifindex", address("239.250.33.1"),
			interface_addr, ifindex, true);
	affected = probe("case_address_only", address("239.250.33.2"),
			 interface_addr, ifindex, false);
	batch = accumulate_groups(interface_addr, ifindex);
	pass = control.before == 0 && control.after_join == 1 &&
	       control.after_drop == 0 && control.after_close == 0 &&
	       affected.before == 0 && affected.after_join == 1 &&
	       affected.after_drop == expect_leak &&
	       affected.after_close == expect_leak &&
	       batch.before == 0 && batch.joins == BATCH_GROUPS &&
	       batch.drops == BATCH_GROUPS && batch.closes == BATCH_GROUPS &&
	       batch.after == (expect_leak ? BATCH_GROUPS : 0);
	printf("result=%s\n", pass ? "PASS" : "FAIL");
	return pass ? EXIT_SUCCESS : EXIT_FAILURE;
}

/*
 * The caller first adds 192.0.2.1/24 to eth0, then 192.0.2.10/24 as a
 * secondary address. After "ready_to_remove_address=1", it deletes only
 * the secondary address and sends one byte on stdin; eth0 remains up.
 */
static int run_address_churn(bool expect_leak, unsigned int ifindex)
{
	struct in_addr group = address("239.250.35.1");
	struct ip_mreq request = {
		.imr_multiaddr = group,
		.imr_interface = address("192.0.2.10"),
	};
	int before, after_join, after_remove, after_drop, after_close, fd;
	struct interface_state before_state, after_state;
	char token;
	ssize_t n;
	bool pass;

	printf("case_address_churn\n");
	before_state = churn_interface_state();
	printf("address_present_before=%d\ninterface_up_before=%d\n",
	       before_state.additional_present, before_state.up);
	before = group_present(ifindex, group);
	printf("before=%d\n", before);
	fd = socket(AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
	if (fd < 0)
		die("socket");
	printf("socket=%d\n", fd);
	if (setsockopt(fd, IPPROTO_IP, IP_ADD_MEMBERSHIP,
		       &request, sizeof(request)))
		die("address-churn IP_ADD_MEMBERSHIP");
	printf("join=0\n");
	after_join = group_present(ifindex, group);
	printf("after_join=%d\n", after_join);
	printf("ready_to_remove_address=1\n");
	if (fflush(stdout))
		die("fflush");
	do {
		n = read(STDIN_FILENO, &token, 1);
	} while (n < 0 && errno == EINTR);
	if (n != 1) {
		fprintf(stderr, "expected one-byte address-removal signal\n");
		return EXIT_FAILURE;
	}
	after_state = churn_interface_state();
	printf("address_removed=%d\ninterface_up=%d\nprimary_present=%d\n",
	       !after_state.additional_present, after_state.up,
	       after_state.primary_present);
	after_remove = group_present(ifindex, group);
	printf("after_remove=%d\n", after_remove);
	if (setsockopt(fd, IPPROTO_IP, IP_DROP_MEMBERSHIP,
		       &request, sizeof(request)))
		die("address-churn IP_DROP_MEMBERSHIP");
	printf("drop=0\n");
	after_drop = group_present(ifindex, group);
	printf("after_drop=%d\n", after_drop);
	if (close(fd))
		die("close");
	printf("close=0\n");
	after_close = group_present(ifindex, group);
	printf("after_close=%d\n", after_close);
	pass = before_state.up && before_state.primary_present &&
	       before_state.additional_present && after_state.up &&
	       after_state.primary_present && !after_state.additional_present &&
	       before == 0 && after_join == 1 && after_remove == 1 &&
	       after_drop == expect_leak && after_close == expect_leak;
	printf("result=%s\n", pass ? "PASS" : "FAIL");
	return pass ? EXIT_SUCCESS : EXIT_FAILURE;
}

int main(int argc, char **argv)
{
	bool churn = argc == 3 && !strcmp(argv[1], "--address-churn");
	const char *expectation = churn ? argv[2] : argc == 2 ? argv[1] : NULL;
	const char *ifname = churn ? "eth0" : "lo";
	bool expect_leak;
	unsigned int ifindex;

	if (!expectation ||
	    (strcmp(expectation, "--expect-leak") &&
	     strcmp(expectation, "--expect-fixed"))) {
		fprintf(stderr,
			"usage: %s [--address-churn] --expect-leak|--expect-fixed\n",
			argv[0]);
		return 2;
	}
	expect_leak = !strcmp(expectation, "--expect-leak");
	ifindex = if_nametoindex(ifname);
	if (!ifindex)
		die("if_nametoindex");
	printf("uid=%u\ninterface=%s ifindex=%u\nexpectation=%s\n",
	       (unsigned int)getuid(), ifname, ifindex,
	       expect_leak ? "leak" : "fixed");

	if (churn)
		return run_address_churn(expect_leak, ifindex);
	return run_original(expect_leak, ifindex);
}

 net/ipv4/igmp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 144fca158..52cde8782 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -2463,6 +2463,8 @@ int ip_mc_leave_group(struct sock *sk, struct ip_mreqn *imr)
 				iml->multi.imr_address.s_addr)
 			continue;
 
+		/* The drop request may not resolve to the joined device. */
+		in_dev = inetdev_by_index(net, iml->multi.imr_ifindex);
 		(void) ip_mc_leave_src(sk, iml, in_dev);
 
 		*imlp = iml->next_rcu;
-- 
2.43.0

             reply	other threads:[~2026-11-09 12:53 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-09 12:53 Junyoung Jang [this message]
2026-11-09 12:55 ` netdev-bot+sinfo
2026-11-09 13:08   ` Junyoung Jang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261109125312.2208874-1-graypanda.inzag@gmail.com \
    --to=graypanda.inzag@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@kernel.org \
    --cc=horms@kernel.org \
    --cc=idosch@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®