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

[Paolo Abeni <pabeni@redhat.com>]

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@gmail.com>
> 
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> 
> 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