Re: [PATCH net] igmp: use the joined device when leaving a multicast group

[Paolo Abeni]

On 11/12/26 09:51, Ido Schimmel wrote:
> On Mon, Nov 09, 2026 at 09:53:11PM +0900, Junyoung Jang wrote:
> > 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@xxxxxxxxx>
>
> Reviewed-by: Ido Schimmel <idosch@xxxxxxxxxx>
>
> IMO, the commit message is close to being unreadable. Simpler (don't
> respin):
>
> "
> When a socket joins a multicast group, the ifindex of the device is
> stored in the socket's membership entry. When leaving,
> ip_mc_leave_group() resolves the device again from the request. If the
> request only specifies an interface address and that address is not
> assigned to any device, no device is found. The membership is still
> matched by address and removed from the socket, but the group is not
> released on the device it was joined on.
>
> The group is then leaked on the device until the device is unregistered.
> An unprivileged user can trigger this by joining with an ifindex and a
> non-local address and leaving with the address only. It can also happen
> when a socket joins and leaves with the address only and the address is
> removed in between.
>
> Fix by resolving the device from the ifindex stored in the membership,
> like ip_mc_drop_socket() does.
> "

Many thanks Ido! I agree with what you wrote. I'll update the commit message
while applying the patch.

/P