DF-3281 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 | --- if.c.orig 2026-09-10 02:34:29.419130783 +0000 +++ if.c.fixed 2026-09-10 02:35:58.185986949 +0000 @@ -1102,19 +1102,34 @@ /* * Create an empty interface group. */ +/* + * NOTE: ifgroup_lock must be held exclusively when calling this. + */ static struct ifg_group * -if_creategroup(const char *groupname) +if_creategroup_locked(const char *groupname) { struct ifg_group *ifg; + KKASSERT(lockstatus(&ifgroup_lock, curthread) == LK_EXCLUSIVE); + ifg = kmalloc(sizeof(*ifg), M_IFNET, M_WAITOK); strlcpy(ifg->ifg_group, groupname, sizeof(ifg->ifg_group)); ifg->ifg_refcnt = 0; ifg->ifg_carp_demoted = 0; TAILQ_INIT(&ifg->ifg_members); - ifgroup_lockmgr(LK_EXCLUSIVE); TAILQ_INSERT_TAIL(&ifg_head, ifg, ifg_next); + + return (ifg); +} + +static struct ifg_group * +if_creategroup(const char *groupname) +{ + struct ifg_group *ifg; + + ifgroup_lockmgr(LK_EXCLUSIVE); + ifg = if_creategroup_locked(groupname); ifgroup_lockmgr(LK_RELEASE); EVENTHANDLER_INVOKE(group_attach_event, ifg); @@ -1148,21 +1163,32 @@ int if_addgroup(struct ifnet *ifp, const char *groupname) { - struct ifg_list *ifgl; + struct ifg_list *ifgl, *ifgl_tmp; struct ifg_group *ifg; struct ifg_member *ifgm; + int created = 0; + int error; if (groupname[0] && groupname[strlen(groupname) - 1] >= '0' && groupname[strlen(groupname) - 1] <= '9') return (EINVAL); - ifgroup_lockmgr(LK_SHARED); + ifgl = kmalloc(sizeof(*ifgl), M_IFNET, M_WAITOK); + ifgm = kmalloc(sizeof(*ifgm), M_IFNET, M_WAITOK); - TAILQ_FOREACH(ifgl, &ifp->if_groups, ifgl_next) { - if (strcmp(ifgl->ifgl_group->ifg_group, groupname) == 0) { - ifgroup_lockmgr(LK_RELEASE); - return (EEXIST); + ifgroup_lockmgr(LK_EXCLUSIVE); + + /* + * NOTE: Group lookup, creation, and the refcount bump are + * performed atomically w.r.t. other if_addgroup() calls and + * w.r.t. if_delgroup_locked() -- ifg_refcnt is modified by + * both, so the increment must be covered by the ifgroup lock. + */ + TAILQ_FOREACH(ifgl_tmp, &ifp->if_groups, ifgl_next) { + if (strcmp(ifgl_tmp->ifgl_group->ifg_group, groupname) == 0) { + error = EEXIST; + goto failed; } } @@ -1170,26 +1196,31 @@ if (strcmp(ifg->ifg_group, groupname) == 0) break; } + if (ifg == NULL) { + ifg = if_creategroup_locked(groupname); + created = 1; + } - ifgroup_lockmgr(LK_RELEASE); - - if (ifg == NULL) - ifg = if_creategroup(groupname); - - ifgl = kmalloc(sizeof(*ifgl), M_IFNET, M_WAITOK); - ifgm = kmalloc(sizeof(*ifgm), M_IFNET, M_WAITOK); ifgl->ifgl_group = ifg; ifgm->ifgm_ifp = ifp; ifg->ifg_refcnt++; - ifgroup_lockmgr(LK_EXCLUSIVE); TAILQ_INSERT_TAIL(&ifg->ifg_members, ifgm, ifgm_next); TAILQ_INSERT_TAIL(&ifp->if_groups, ifgl, ifgl_next); + ifgroup_lockmgr(LK_RELEASE); + if (created) + EVENTHANDLER_INVOKE(group_attach_event, ifg); EVENTHANDLER_INVOKE(group_change_event, groupname); return (0); + +failed: + ifgroup_lockmgr(LK_RELEASE); + kfree(ifgl, M_IFNET); + kfree(ifgm, M_IFNET); + return (error); } /* |