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

[henry martin]

Thanks, the Medium finding is legitimate.

The 64-bit claim in v2 is wrong: group IDs are allocated in
contiguous ranges first-fit (genl_allocate_reserve_groups()) and
genl_unregister_mc_groups() only clears bits without compacting, so
with ~59 free IDs against ~54 in-tree dynamic groups, an
unload/reload sequence can fragment the free space below the size a
multi-group family needs, grow mc_groups_longs past 1, and open the
same stale-bitmap overflow on 64-bit kernels with in-tree modules
only.  Only the privileged module (un)loading requirement is shared
with the 32-bit path.

v3 rewords the reachability paragraph accordingly (guard unchanged);
Cc: stable stays dropped per the earlier "no reproducer" verdict,
maintainers' call.

pw-bot: cr

Regards,
Henry

<netdev-bot+sashiko@xxxxxxxxxx> 于2026年10月14日周三 10:40写道：
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 1 · 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).
>
> - [Medium] The code guard is correct.
>
> --
>
> netlink: fix out-of-bounds bitmap access clearing stale mc groups
>
> This patch adds a bounds check to netlink_update_socket_mc().  With it,
> __netlink_clear_multicast_users() no longer tests or clears bits past a
> socket's nlk->groups bitmap.  Sockets whose bitmap was sized before the
> multicast group table grew are skipped for groups beyond nlk->ngroups.
>
> > With in-tree families the overflow is only reachable on 32-bit
> > kernels: the ~54 dynamically allocated genl groups fit the initial
> > 64-bit bitmap on 64-bit kernels but exceed the 32-bit one, so the
> > table can grow there, and a later module unload then touches bits
> > past the bitmap of stale sockets.
>
> [Severity: Medium]
> The guard looks correct on every architecture.  Is the claim that only
> 32-bit kernels are affected accurate, though?
>
> genl_allocate_reserve_groups() in net/netlink/genetlink.c does a first-fit
> search for a contiguous range.  It grows the table whenever that range ends
> past the current capacity:
>
> net/netlink/genetlink.c:genl_allocate_reserve_groups() {
>     ...
>                 if (id + n_groups > mc_groups_longs * BITS_PER_LONG) {
>                         unsigned long new_longs = mc_groups_longs +
>                                                   BITS_TO_LONGS(n_groups);
>     ...
> }
>
> genl_unregister_mc_groups() frees IDs with clear_bit(), and nothing
> compacts the remaining allocations.  So the number of live groups does not
> limit the highest group ID.
>
> The initial bitmap also starts with five bits reserved:
>
> static unsigned long mc_group_start = 0x3 | BIT(GENL_ID_CTRL) |
>                                       BIT(GENL_ID_VFS_DQUOT) |
>                                       BIT(GENL_ID_PMCRAID);
>
> That leaves 59 free IDs on a 64-bit kernel, only about 5 more than the ~54
> in-tree groups.
>
> With most in-tree families loaded, can an unload/reload sequence fragment
> the free space enough to force growth on 64-bit?  For example:
>
>   unload nl80211 (7 groups with CONFIG_NL80211_TESTMODE) and psample
>   load psample, which takes part of nl80211's old range
>   load nl80211, which no longer fits in any free 7-bit gap
>
> At that point id + n_groups > 64, mc_groups_longs becomes 2, and
> __netlink_change_ngroups() raises tbl->groups to 128.  Sockets that
> subscribed earlier still have nlk->ngroups == 64 and an 8-byte bitmap.
>
> A later unload of nl80211 then reaches:
>
> genl_unregister_mc_groups()
>   __netlink_clear_multicast_users()
>     netlink_update_socket_mc()
>       test_bit(group - 1, nlk->groups)        /* bit 64 */
>       __assign_bit(group - 1, nlk->groups, 0)
>
> Without this patch, that would be an out-of-bounds access on a 64-bit
> kernel with only in-tree modules.
>
> The v2 notes say Cc: stable was dropped because of this reachability
> description.  Could this paragraph be reworded so backporters don't
> conclude that 64-bit kernels are unaffected?
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261013023859.3075601-1-bsdhenrymartin%40gmail.com