Start refcounting interface groups with 1. if_creategroup() returns

a new object that is already refcounted, so carp attach does not
reach into internal structures.  Add kasserts to detect counter
overflow or underflow.
OK mvs@

2 files changed, 15 insertions, 13 deletions

diff --git a/sys/net/if.c b/sys/net/if.c
index 4be2af461df..6b22927d48b 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if.c,v 1.626 2021/02/01 07:43:33 mvs Exp $	*/
+/*	$OpenBSD: if.c,v 1.627 2021/02/08 12:30:10 bluhm Exp $	*/
 /*	$NetBSD: if.c,v 1.35 1996/05/07 05:26:04 thorpej Exp $	*/
 
 /*
@@ -2601,7 +2601,7 @@ if_creategroup(const char *groupname)
 		return (NULL);
 
 	strlcpy(ifg->ifg_group, groupname, sizeof(ifg->ifg_group));
-	ifg->ifg_refcnt = 0;
+	ifg->ifg_refcnt = 1;
 	ifg->ifg_carp_demoted = 0;
 	TAILQ_INIT(&ifg->ifg_members);
 #if NPF > 0
@@ -2642,13 +2642,17 @@ if_addgroup(struct ifnet *ifp, const char *groupname)
 		if (!strcmp(ifg->ifg_group, groupname))
 			break;
 
-	if (ifg == NULL && (ifg = if_creategroup(groupname)) == NULL) {
-		free(ifgl, M_TEMP, sizeof(*ifgl));
-		free(ifgm, M_TEMP, sizeof(*ifgm));
-		return (ENOMEM);
-	}
+	if (ifg == NULL) {
+		ifg = if_creategroup(groupname);
+		if (ifg == NULL) {
+			free(ifgl, M_TEMP, sizeof(*ifgl));
+			free(ifgm, M_TEMP, sizeof(*ifgm));
+			return (ENOMEM);
+		}
+	} else
+		ifg->ifg_refcnt++;
+	KASSERT(ifg->ifg_refcnt != 0);
 
-	ifg->ifg_refcnt++;
 	ifgl->ifgl_group = ifg;
 	ifgm->ifgm_ifp = ifp;
 
@@ -2692,6 +2696,7 @@ if_delgroup(struct ifnet *ifp, const char *groupname)
 	pfi_group_change(groupname);
 #endif
 
+	KASSERT(ifgl->ifgl_group->ifg_refcnt != 0);
 	if (--ifgl->ifgl_group->ifg_refcnt == 0) {
 		TAILQ_REMOVE(&ifg_head, ifgl->ifgl_group, ifg_next);
 #if NPF > 0
diff --git a/sys/netinet/ip_carp.c b/sys/netinet/ip_carp.c
index 00d5fe91512..15593a5fbf0 100644
--- a/sys/netinet/ip_carp.c
+++ b/sys/netinet/ip_carp.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_carp.c,v 1.351 2021/01/21 13:18:07 mvs Exp $	*/
+/*	$OpenBSD: ip_carp.c,v 1.352 2021/02/08 12:30:10 bluhm Exp $	*/
 
 /*
  * Copyright (c) 2002 Michael Shalayeff. All rights reserved.
@@ -786,10 +786,7 @@ carp_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
 void
 carpattach(int n)
 {
-	struct ifg_group	*ifg;
-
-	if ((ifg = if_creategroup("carp")) != NULL)
-		ifg->ifg_refcnt++;	/* keep around even if empty */
+	if_creategroup("carp");  /* keep around even if empty */
 	if_clone_attach(&carp_cloner);
 	carpcounters = counters_alloc(carps_ncounters);
 }