[PATCH net v3] netlink: fix out-of-bounds bitmap access clearing stale mc groups

[PATCH net v3] netlink: fix out-of-bounds bitmap access clearing stale mc groups

[Henry Martin]

netlink_realloc_groups() sizes nlk->groups to the number of groups
that exist at bind/ADD_MEMBERSHIP time.  When more multicast groups
are registered later (e.g. a new genl family), existing sockets keep
their smaller bitmap.

When clearing a multicast group of a departing family,
__netlink_clear_multicast_users() runs netlink_update_socket_mc() on
each socket on mc_list, which does test_bit()/__assign_bit() on
group - 1 with no regard to nlk->ngroups.  Skip groups that lie
beyond the socket's bitmap: such a socket could never have joined
them.  This is the same guard do_one_broadcast() already applies.

The overflow windows differ per architecture, both needing privileged
module (un)loading.  On 32-bit kernels the ~54 dynamically allocated
genl groups already exceed the initial 32-bit bitmap, so the table
can grow there directly.  On 64-bit kernels the same in-tree set
fits the initial 64-bit bitmap only while allocation stays
contiguous: genl_allocate_reserve_groups() takes the first free
contiguous range and unregister does not compact, so an
unload/reload sequence can fragment the ~59 free IDs until no
fitting range remains, forcing the table past 64 bits and reaching
the same stale-bitmap overflow on the next family teardown.

This issue was discovered by Tencent CodeBuddy Security.

Fixes: 84659eb529b3 ("[NETLIKN]: Allow removing multicast groups.")
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---

v3: Correct the reachability again (sashiko review): 64-bit is also
    reachable - group IDs are allocated in contiguous ranges without
    compaction, so fragmentation from an unload/reload sequence can
    force the table past the initial 64-bit bitmap too.

v2: Correct the Fixes tag (the clear path was added by 84659eb529b3,
    not by the dynamic bitmap commit), drop Cc: stable and describe
    the actual reachability (32-bit kernels with in-tree families)
    instead of implying generic heap corruption.

 net/netlink/af_netlink.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 9fdf964224ab4..8cc655aa927f1 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1657,6 +1657,12 @@ static void netlink_update_socket_mc(struct netlink_sock *nlk,
 {
 	int old, new = !!is_new, subscriptions;
 
+	/* A socket whose bitmap predates this group can never be a member;
+	 * don't touch bits beyond its allocation.
+	 */
+	if (group - 1 >= nlk->ngroups)
+		return;
+
 	old = test_bit(group - 1, nlk->groups);
 	subscriptions = nlk->subscriptions - old + new;
 	__assign_bit(group - 1, nlk->groups, new);
-- 
2.43.7

Re: [PATCH net v3] netlink: fix out-of-bounds bitmap access clearing stale mc groups

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v3] netlink: fix out-of-bounds bitmap access clearing stale mc groups

[Jakub Kicinski]

On Tue, 20 Oct 2026 11:21:19 +0800 Henry Martin wrote:
> netlink_realloc_groups() sizes nlk->groups to the number of groups
> that exist at bind/ADD_MEMBERSHIP time.  When more multicast groups
> are registered later (e.g. a new genl family), existing sockets keep
> their smaller bitmap.
> 
> When clearing a multicast group of a departing family,
> __netlink_clear_multicast_users() runs netlink_update_socket_mc() on
> each socket on mc_list, which does test_bit()/__assign_bit() on
> group - 1 with no regard to nlk->ngroups.  Skip groups that lie
> beyond the socket's bitmap: such a socket could never have joined
> them.  This is the same guard do_one_broadcast() already applies.
> 
> The overflow windows differ per architecture, both needing privileged
> module (un)loading.  On 32-bit kernels the ~54 dynamically allocated
> genl groups already exceed the initial 32-bit bitmap, so the table
> can grow there directly.  On 64-bit kernels the same in-tree set
> fits the initial 64-bit bitmap only while allocation stays
> contiguous: genl_allocate_reserve_groups() takes the first free
> contiguous range and unregister does not compact, so an
> unload/reload sequence can fragment the ~59 free IDs until no
> fitting range remains, forcing the table past 64 bits and reaching
> the same stale-bitmap overflow on the next family teardown.
> 
> This issue was discovered by Tencent CodeBuddy Security.
> 
> Fixes: 84659eb529b3 ("[NETLIKN]: Allow removing multicast groups.")
> Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
> ---
> 
> v3: Correct the reachability again (sashiko review): 64-bit is also
>     reachable - group IDs are allocated in contiguous ranges without
>     compaction, so fragmentation from an unload/reload sequence can
>     force the table past the initial 64-bit bitmap too.
> 
> v2: Correct the Fixes tag (the clear path was added by 84659eb529b3,
>     not by the dynamic bitmap commit), drop Cc: stable and describe
>     the actual reachability (32-bit kernels with in-tree families)
>     instead of implying generic heap corruption.
> 
>  net/netlink/af_netlink.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index 9fdf964224ab4..8cc655aa927f1 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -1657,6 +1657,12 @@ static void netlink_update_socket_mc(struct netlink_sock *nlk,
>  {
>  	int old, new = !!is_new, subscriptions;
>  
> +	/* A socket whose bitmap predates this group can never be a member;
> +	 * don't touch bits beyond its allocation.
> +	 */
> +	if (group - 1 >= nlk->ngroups)
> +		return;
> +
>  	old = test_bit(group - 1, nlk->groups);
>  	subscriptions = nlk->subscriptions - old + new;
>  	__assign_bit(group - 1, nlk->groups, new);

Dunno :/ I would prefer the following fix, otherwise it looks like we
may silently ignore a netlink_update_socket_mc(..., 1)

@@ -2163,8 +2157,12 @@ void __netlink_clear_multicast_users(struct sock *ksk, unsigned int group)
        struct netlink_table *tbl = &nl_table[ksk->sk_protocol];
        struct hlist_node *tmp;
 
-       sk_for_each_bound_safe(sk, tmp, &tbl->mc_list)
+       sk_for_each_bound_safe(sk, tmp, &tbl->mc_list) {
+               /* bitmap predates the group, can't be a member */
+               if (group - 1 >= nlk_sk(sk)->ngroups)
+                       continue;
                netlink_update_socket_mc(nlk_sk(sk), group, 0);
+       }
 }
-- 
pw-bot: cr