Re: [PATCH net v6] genetlink: Reject joining multicast groups no family owns

[Nicolas Dichtel]

Le 12/11/2026 à 10:42, Anil Kaushik a écrit :
> genl_bind() checks the GENL_MCAST_CAP_* flags of a group only when a
> family owns it. For a group ID that no family owns, it returns 0, so any
> user can join it.
> 
> A family registered later can get that ID, and the socket stays a member:
> nothing checks it again. An unprivileged user can join the free IDs, load
> a family module with CTRL_CMD_GETFAMILY, which calls request_module(),
> and then receive the notifications of a group with
> GENL_MCAST_CAP_NET_ADMIN or GENL_MCAST_CAP_SYS_ADMIN. For example the
> drop_monitor "events" group always gets ID 1, and is free until the
> drop_monitor module is loaded.
> 
> Fix by returning -ENOENT from genl_bind() when no family owns the group.
> There are no group flags to check before the family registers, so this
> applies to privileged callers too: a program that joins a fixed ID such
> as NET_DM_GRP_ALERT before the family is loaded no longer gets its
> messages. I don't know of a program that does this. One that resolves
> the family first still joins.
> 
> bind() takes a mask of groups, and most IDs in it can have no owner. Such
> a mask used to work, so keep it working: netlink_bind() skips an ID when
> the bind callback returns -ENOENT, and joins the others. Only genl_bind()
> returns -ENOENT. A skipped ID is not joined, so it is no longer in the
> groups that getsockname() and NETLINK_LIST_MEMBERSHIPS report.
> setsockopt() returns the error, as it asks for one group.
> 
> This does not cover a family that is unregistered between the check and
> the point where the membership bit is set.
> 
> Found by the Sashiko review of the gtp multicast group fix. Tested as an
> unprivileged user who joins all free IDs, then loads gtp and
> drop_monitor:
> 
> Before:
>   joins: ok=62 EPERM=2
>   gtp/gtp (ID 15): member
>   NET_DM/events (ID 1): member
> 
> After:
>   joins: ok=19 ENOENT=43 EPERM=2
>   gtp/gtp (ID 15): not member
>   NET_DM/events (ID 1): not member
>   bind(), an owned group and free IDs: ok, member of the owned group only
> 
> genl_bind() was added, with its first capability check, in the commit
> in the Fixes tag.
> 
> Link: https://lore.kernel.org/netdev/179324692107.3134.14443689892607921337@xxxxxxxxxx/
> Fixes: 4d54cc32112d ("mptcp: avoid lock_fast usage in accept path")
> Signed-off-by: Anil Kaushik <anilkaushikwireless@xxxxxxxxx>
> ---
> v6:
> - Drop patch 2 of v5, as Jakub found it too ugly. A family unregistered
>   between the check and setting the membership bit is not covered.
> - Say that early joins to fixed IDs stop working, also for privileged
>   callers, and that NETLINK_LIST_MEMBERSHIPS changes.
> - Document the -ENOENT meaning on the bind member.
> - Rebase on net/main.
> 
> v5: https://lore.kernel.org/netdev/20261104092125.211537-1-anilkaushikwireless@xxxxxxxxx/
> 
> I used an LLM to help write and review the code, the tests and the
> commit message of this patch. I checked and tested every change myself.
Could these tests be added in selftests?