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

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

[Anil Kaushik]

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.
Nothing is ever sent to such a group, so joining it has no use.

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

Link: https://lore.kernel.org/netdev/179324692107.3134.14443689892607921337@kernel.org/
Fixes: e03781879a0d ("drop_monitor: Require 'CAP_SYS_ADMIN' when joining "events" group")
Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>
---

Notes:
    The gtp patch: https://lore.kernel.org/netdev/20261027130623.3136658-1-anilkaushikwireless@gmail.com/

 net/netlink/genetlink.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 5cc1037d4917..f038f04ef4ea 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -1825,7 +1825,7 @@ static int genl_bind(struct net *net, int group)
 {
 	const struct genl_family *family;
 	unsigned int id;
-	int ret = 0;
+	int ret = -ENOENT;
 
 	down_read(&cb_lock);
 
@@ -1841,6 +1841,7 @@ static int genl_bind(struct net *net, int group)
 			continue;
 
 		grp = &family->mcgrps[i];
+		ret = 0;
 		if ((grp->flags & GENL_MCAST_CAP_NET_ADMIN) &&
 		    !ns_capable(net->user_ns, CAP_NET_ADMIN))
 			ret = -EPERM;
-- 
2.25.1

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

[Anil Kaushik]

Adding Jacob and Jiri, I missed them on Cc.

On Thu, Oct 29, 2026, Anil Kaushik wrote:
> 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.

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

[Nicolas Dichtel]

Le 29/10/2026 à 07:22, 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.
> Nothing is ever sent to such a group, so joining it has no use.
Moreover, a user should retrieve the group index before joining it. It seems
weird to join it without this step.

> 
> 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
> 
> Link: https://lore.kernel.org/netdev/179324692107.3134.14443689892607921337@kernel.org/
> Fixes: e03781879a0d ("drop_monitor: Require 'CAP_SYS_ADMIN' when joining "events" group")
> Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>

Reviewed-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

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

[Ido Schimmel]

On Thu, Oct 29, 2026 at 06:22:14AM +0000, Anil Kaushik wrote:
> 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.
> Nothing is ever sent to such a group, so joining it has no use.
> 
> 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
> 
> Link: https://lore.kernel.org/netdev/179324692107.3134.14443689892607921337@kernel.org/
> Fixes: e03781879a0d ("drop_monitor: Require 'CAP_SYS_ADMIN' when joining "events" group")
> Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>

FWIW, this restores the behavior of commit 5ad6300524c0 ("genetlink:
disallow subscribing to unknown mcast groups"). The entire function was
later removed by commit 1e82a62fec61 ("genetlink: remove genl_bind") for
unrelated reasons.

So, given that the kernel already rejected such bindings between v3.19
and v5.7, I think that the potential for regressions is low.

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix is incomplete because of a check-then-use race.

--

genetlink: Reject joining multicast groups no family owns

This patch makes genl_bind() return -ENOENT when no registered family
owns the requested multicast group ID. Before the patch such joins
succeeded. If a family with GENL_MCAST_CAP_* flags was later given that
ID, the socket stayed a member.

Link: https://lore.kernel.org/netdev/179324692107.3134.14443689892607921337@kernel.org/

> diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
> index 5cc1037d4917e..f038f04ef4ead 100644
> --- a/net/netlink/genetlink.c
> +++ b/net/netlink/genetlink.c
> @@ -1825,7 +1825,7 @@ static int genl_bind(struct net *net, int group)
>  {
>  	const struct genl_family *family;
>  	unsigned int id;
> -	int ret = 0;
> +	int ret = -ENOENT;
>  
>  	down_read(&cb_lock);
>  
> @@ -1841,6 +1841,7 @@ static int genl_bind(struct net *net, int group)
>  			continue;
>  
>  		grp = &family->mcgrps[i];
> +		ret = 0;
>  		if ((grp->flags & GENL_MCAST_CAP_NET_ADMIN) &&
>  		    !ns_capable(net->user_ns, CAP_NET_ADMIN))
>  			ret = -EPERM;

[Severity: High]
Does this fully close the case described in the commit message?

genl_bind() checks ownership and the GENL_MCAST_CAP_* flags under
down_read(&cb_lock). It drops the lock before it returns. The caller
sets the membership bit only afterwards:

net/netlink/af_netlink.c:netlink_setsockopt() {
    ...
		if (optname == NETLINK_ADD_MEMBERSHIP && nlk->netlink_bind) {
			err = nlk->netlink_bind(sock_net(sk), val);
			if (err)
				return err;
		}
		netlink_table_grab();
		netlink_update_socket_mc(nlk, val,
					 optname == NETLINK_ADD_MEMBERSHIP);
		netlink_table_ungrab();
    ...
}

netlink_bind() follows the same pattern. It updates nlk->groups[0] under
netlink_table_grab() after genl_bind() has returned.

The owning family can be unregistered inside that window, for example by
a module unload or by a module init that fails after
genl_register_family(). Can the following sequence happen?

CPU1 (unprivileged)                  CPU2
netlink_setsockopt(ADD_MEMBERSHIP, G)
  genl_bind()
    down_read(&cb_lock)
    family F owns G, no caps, ret = 0
    up_read(&cb_lock)
                                     genl_unregister_family(F)
                                       down_write(&cb_lock)
                                       genl_unregister_mc_groups()
                                         __netlink_clear_multicast_users()
                                           (CPU1 not a member yet)
                                         clear_bit(G, mc_groups)
                                       idr_remove()
  netlink_table_grab()
  netlink_update_socket_mc(nlk, G, 1)

After this sequence the socket is a member of a group ID that no family
owns.

Later, genl_register_family()->genl_validate_assign_mc_groups()->
genl_allocate_reserve_groups() can give G to a family whose group has
GENL_MCAST_CAP_NET_ADMIN or GENL_MCAST_CAP_SYS_ADMIN, such as psample,
mptcp pm, ioam6 or cifs. Nothing clears the existing members of G at
that point. do_one_broadcast() delivers based only on the membership
bit. The CTRL_CMD_GETFAMILY->request_module() autoload path from the
commit message could still trigger that registration.

That is the end state the commit message says is fixed:

  "A family registered later can get that ID, and the socket stays a
  member: nothing checks it again."

Would it be more robust to clear existing members of newly assigned
group IDs in genl_validate_assign_mc_groups()? That could run under
netlink_table_grab(), the same way genl_unregister_mc_groups() clears
them on removal. It would cover both the original case and this race,
instead of relying on the check in genl_bind().

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261029062214.3407223-1-anilkaushikwireless%40gmail.com