Bug 290893 - netlink: genl_register_family function does not release the lock if the family name is already in use
Summary: netlink: genl_register_family function does not release the lock if the famil...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 15.0-CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-net (Nobody)
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2025-11-08 12:26 UTC by Bruno Silvestre
Modified: 2025-11-10 19:01 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Bruno Silvestre 2025-11-08 12:26:54 UTC
The function genl_register_family() returns an error if the name of the family is already in use, but does not release the lock.
The next call to genl_register_family() will block indefinitely.

A side comment, why does the function panic if the maximum number of families are already registered? It could just return an error.

--- netlink_generic.old.c       2025-11-08 08:55:46.012479000 -0300
+++ netlink_generic.c   2025-11-08 09:00:00.541146000 -0300
@@ -366,8 +366,10 @@
        GENL_LOCK();
        for (u_int i = 0; i < MAX_FAMILIES; i++)
                if (families[i].family_name != NULL &&
-                   strcmp(families[i].family_name, family_name) == 0)
+                   strcmp(families[i].family_name, family_name) == 0) {
+                       GENL_UNLOCK();
                        return (0);
+               }

        /* Microoptimization: index 0 is reserved for the control family. */
        gf = NULL;
@@ -376,6 +378,10 @@
                        gf = &families[i];
                        break;
                }
+
+       //
+       //XXX: Why kernel panic? It could return 0 as register group below.
+       //
        KASSERT(gf, ("%s: maximum of %u generic netlink families allocated",
            __func__, MAX_FAMILIES));


//----------------------------------------------------------------------------

To test the bug, just creates a module that register a family.
You load and unload it 3 times: 1st ok, 2nd error, 3rd deadlock.

#include <sys/param.h>
#include <sys/module.h>
#include <sys/kernel.h>
#include <sys/systm.h>
#include <sys/malloc.h>

#include <netlink/netlink.h>
#include <netlink/netlink_ctl.h>
#include <netlink/netlink_generic.h>


static int
genltest_modevent(module_t mod __unused, int event, void *arg __unused)
{
        int error = 0;
        switch (event) {
        case MOD_LOAD:
                uprintf("Generic netlink load\n");
                uint16_t id = genl_register_family("genltest", 0, 2, 0);
                uprintf("Register: %d\n", id);
                break;
        case MOD_UNLOAD:
                uprintf("Generic netlink unload\n");
                break;
        default:
                error = EOPNOTSUPP;
                break;
        }
        return (error);
}

static moduledata_t genltest_mod = {
        "genltest",
        genltest_modevent,
        NULL
};

DECLARE_MODULE(genltest, genltest_mod, SI_SUB_DRIVERS, SI_ORDER_MIDDLE);
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2025-11-08 15:55:03 UTC
^Triage: set patch keyword to denote 'inline patch'.  (These days we would rather
see them as attachments.)

While here, note that this patch applies to sys/netlink/netlink_generic.c .
Comment 2 commit-hook freebsd_committer freebsd_triage 2025-11-08 18:03:40 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=335fc09ab8d25c3ced027d46f5a0f4103d5c8bee

commit 335fc09ab8d25c3ced027d46f5a0f4103d5c8bee
Author:     Bruno Silvestre <bruno.silvestre@gmail.com>
AuthorDate: 2025-11-08 18:02:32 +0000
Commit:     Gleb Smirnoff <glebius@FreeBSD.org>
CommitDate: 2025-11-08 18:02:32 +0000

    netlink: plug lock leak in genl_register_family()

    PR:             290893

 sys/netlink/netlink_generic.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
Comment 3 commit-hook freebsd_committer freebsd_triage 2025-11-10 17:37:46 UTC
A commit in branch stable/15 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=a3cd3a4fd68c8a2ea9264f168441a2ce7d51b4c8

commit a3cd3a4fd68c8a2ea9264f168441a2ce7d51b4c8
Author:     Bruno Silvestre <bruno.silvestre@gmail.com>
AuthorDate: 2025-11-08 18:02:32 +0000
Commit:     Gleb Smirnoff <glebius@FreeBSD.org>
CommitDate: 2025-11-10 17:36:35 +0000

    netlink: plug lock leak in genl_register_family()

    PR:             290893
    (cherry picked from commit 335fc09ab8d25c3ced027d46f5a0f4103d5c8bee)

 sys/netlink/netlink_generic.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
Comment 4 commit-hook freebsd_committer freebsd_triage 2025-11-10 18:54:05 UTC
A commit in branch releng/15.0 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=40056e8abc4261651991320488d5e9a1068e6888

commit 40056e8abc4261651991320488d5e9a1068e6888
Author:     Bruno Silvestre <bruno.silvestre@gmail.com>
AuthorDate: 2025-11-08 18:02:32 +0000
Commit:     Colin Percival <cperciva@FreeBSD.org>
CommitDate: 2025-11-10 18:49:11 +0000

    netlink: plug lock leak in genl_register_family()

    Approved by:    re (cperciva)
    PR:             290893
    (cherry picked from commit 335fc09ab8d25c3ced027d46f5a0f4103d5c8bee)
    (cherry picked from commit a3cd3a4fd68c8a2ea9264f168441a2ce7d51b4c8)

 sys/netlink/netlink_generic.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)