Re: git: 926d2eadcb67 - main - netlink: some refactoring of NETLINK_GENERIC layer

From: Mark Johnston <markj_at_freebsd.org>
Date: Sat, 11 Jan 2025 22:21:28 UTC
On Sat, Jan 11, 2025 at 05:00:29AM +0000, Gleb Smirnoff wrote:
> The branch main has been updated by glebius:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=926d2eadcb671dd26431a1082d4c49c3d5ad7f22
> 
> commit 926d2eadcb671dd26431a1082d4c49c3d5ad7f22
> Author:     Gleb Smirnoff <glebius@FreeBSD.org>
> AuthorDate: 2025-01-11 04:59:29 +0000
> Commit:     Gleb Smirnoff <glebius@FreeBSD.org>
> CommitDate: 2025-01-11 04:59:29 +0000
> 
>     netlink: some refactoring of NETLINK_GENERIC layer
>     
>     - Statically initialize control family/group.  This removes extra startup
>     code and provides a strong guarantee that they reside at the 0 index of
>     the respective arrays.  Before a genl_register_family() with a higher
>     SYSINIT order could try to hijack index 0.

This use of index zero breaks nl_isset_group_locked() and related
functions, which subtract one from the index before looking it up in a
bitset.  The subtraction becomes an underflow, so the bitset operation
triggers an OOB memory access.  This can be reproduced by running the
sys/netlink tests with a KASAN kernel.

>     - Remove the family_id field completely.  Now the family ID as well as
>     group ID are array indices and there is basically no place for a mistake.
>     Previous code had a bug where a KPI user could induce an ID mismatch.
>     
>     - Merge netlink_generic_kpi.c to netlink_generic.c.  Both files are small
>     and now there is more dependency between the control family and the family
>     allocator. Ok'ed by melifaro@.
>     
>     Reviewed by:            melifaro
>     Differential Revision:  https://reviews.freebsd.org/D48316