kern/68026: [patch] add net_remove_domain, domain refcounts

Aaron Fabbri fabbri at isilon.com
Thu Jun 17 00:00:45 GMT 2004


>Number:         68026
>Category:       kern
>Synopsis:       [patch] add net_remove_domain, domain refcounts
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          update
>Submitter-Id:   current-users
>Arrival-Date:   Thu Jun 17 00:00:44 GMT 2004
>Closed-Date:
>Last-Modified:
>Originator:     Aaron Fabbri
>Release:        FreeBSD-CURRENT
>Organization:
Isilon Systems	
>Environment:
System: FreeBSD -CURRENT as of Jun 15, i386

>Description:

There is currently a net_add_domain(), but no net_remove_domain(). 
This means you can't safely unload a module which implements a 
protocol domain.

>How-To-Repeat:
n/a

>Fix:

Refcount domains and provide a safe net_remove_domain()
function.

- Add net_ref_domain() and net_unref_domain(). 

- Add new net_remove_domain() function which allows removal of a
domain iff its refcount is < 1.

- Changed pffindtype() and pffindproto() to "pfgettype" and
"pfgetproto".  On success, these functions now ref the domain which
owns the protosw returned.

- Added pfput which unref's the domain which owns the supplied
protosw.

Patch tested on an older branch of FreeBSD (__FreeBSD_version 500028).
This diff is against -current, but I have not compiled/tested it there.	

Patch:


diff -ru ../../freebsd-current/src/sys/kern/uipc_domain.c ./sys/kern/uipc_domain.c
--- ../../freebsd-current/src/sys/kern/uipc_domain.c	2004-04-05 14:03:36.000000000 -0700
+++ ./sys/kern/uipc_domain.c	2004-06-16 15:34:16.000000000 -0700
@@ -70,6 +70,29 @@
 MTX_SYSINIT(domain, &dom_mtx, "domain list", MTX_DEF);
 
 /*
+ * Ref domain.  net_add_domain() adds 1 ref, as do sockets of the domain via
+ * socreate(). 
+ */
+static void
+net_ref_domain(struct domain *dom)
+{
+	mtx_assert(MA_OWNED, &dom_mtx)
+	dom->dom_refcnt++;
+}
+
+/*
+ * Remove ref on domain.  net_remove_domain() removes the last ref.
+ */
+static void 
+net_unref_domain(struct domain *dom)
+{
+	mtx_assert(MA_OWNED, &dom_mtx);
+	if (dom->dom_refcnt <= 0) 
+		panic("Too many unrefs on domain %s.", dom->dom_name);
+	dom->dom_refcnt--;
+}
+
+/*
  * Add a new protocol domain to the list of supported domains
  * Note: you cant unload it again because  a socket may be using it.
  * XXX can't fail at this time.
@@ -110,10 +133,47 @@
 	mtx_lock(&dom_mtx);
 	dp->dom_next = domains;
 	domains = dp;
+	net_ref_domain(dp);
 	mtx_unlock(&dom_mtx);
 	net_init_domain(dp);
 }
 
+/* 
+ * Remove a domain. Returns ENOENT if 'dp' cannot be found, or EBUSY if 'dp' is
+ * in use by a socket. 
+ */
+int 
+net_remove_domain(struct domain *dp)
+{
+	int error = ENOENT;
+	struct domain *idp, *dpprev;
+
+	mtx_lock(&dom_mtx);
+
+	dpprev = NULL;
+	idp = domains;
+	while (idp != NULL) {
+		if (idp == dp) {
+			if (dp->dom_refcnt > 1) {
+				error = EBUSY;
+				break;
+			}
+			if (dpprev == NULL)
+				domains = idp->dom_next;
+			else
+				dpprev->dom_next = idp->dom_next;
+			net_unref_domain(dp);
+			error = 0;
+			break;
+		}
+		dpprev = idp;
+		idp = idp->dom_next;
+	}	
+
+	mtx_unlock(&dom_mtx);
+	return (error);
+}
+
 /* ARGSUSED*/
 static void
 domaininit(void *dummy)
@@ -143,51 +203,99 @@
 }
 
 
+/*
+ * Find a protocol switch by family and type.  If found, ref the domain.
+ */
 struct protosw *
-pffindtype(family, type)
+pfgettype(family, type)
 	int family;
 	int type;
 {
 	register struct domain *dp;
-	register struct protosw *pr;
+	register struct protosw *pr = NULL;
+	int found_fam = 0, found = 0;
+
+	mtx_lock(&dom_mtx);
 
 	for (dp = domains; dp; dp = dp->dom_next)
-		if (dp->dom_family == family)
-			goto found;
-	return (0);
-found:
-	for (pr = dp->dom_protosw; pr < dp->dom_protoswNPROTOSW; pr++)
-		if (pr->pr_type && pr->pr_type == type)
-			return (pr);
-	return (0);
+		if (dp->dom_family == family) {
+			found_fam = 1;
+			break;
+		}
+
+	if (found_fam) {
+		for (pr = dp->dom_protosw; pr < dp->dom_protoswNPROTOSW; pr++)
+			if (pr->pr_type && pr->pr_type == type) {
+				found = 1;
+				break;
+			}
+	}
+	if (found) 
+		net_ref_domain(dp);
+	else
+		pr = NULL;
+
+	mtx_unlock(&dom_mtx);
+	return (pr);
 }
 
+/*
+ * Find a protocol switch by family, protocol and type.  If found, ref the
+ * domain.
+ */
 struct protosw *
-pffindproto(family, protocol, type)
+pfgetproto(family, protocol, type)
 	int family;
 	int protocol;
 	int type;
 {
 	register struct domain *dp;
-	register struct protosw *pr;
+	register struct protosw *pr = NULL;
 	struct protosw *maybe = 0;
+	int found_fam = 0, found = 0;
 
 	if (family == 0)
 		return (0);
+
+	mtx_lock(&dom_mtx);
+
 	for (dp = domains; dp; dp = dp->dom_next)
-		if (dp->dom_family == family)
-			goto found;
-	return (0);
-found:
-	for (pr = dp->dom_protosw; pr < dp->dom_protoswNPROTOSW; pr++) {
-		if ((pr->pr_protocol == protocol) && (pr->pr_type == type))
-			return (pr);
-
-		if (type == SOCK_RAW && pr->pr_type == SOCK_RAW &&
-		    pr->pr_protocol == 0 && maybe == (struct protosw *)0)
-			maybe = pr;
+		if (dp->dom_family == family) {
+			found_fam = 1;
+			break;
+		}
+	
+	if (found_fam) {
+		for (pr = dp->dom_protosw; pr < dp->dom_protoswNPROTOSW; pr++) {
+			if ((pr->pr_protocol == protocol) && 
+			    (pr->pr_type == type)) {
+				found = 1;
+				break;
+			}
+			if (type == SOCK_RAW && pr->pr_type == SOCK_RAW &&
+				pr->pr_protocol == 0 && maybe == (struct protosw *)0)
+				maybe = pr;
+		}
 	}
-	return (maybe);
+
+	if (found) 
+		net_ref_domain(dp);
+	else if (maybe != NULL) {
+		net_ref_domain(dp);
+		pr = maybe;
+	} else 
+		pr = NULL;
+
+	mtx_unlock(&dom_mtx);
+	return (pr);
+}
+
+void
+pfput(struct protosw* pr)
+{
+	mtx_lock(&dom_mtx);
+	net_unref_domain(pr->pr_domain);
+	mtx_unlock(&dom_mtx);
 }
 
 void
diff -ru ../../freebsd-current/src/sys/kern/uipc_socket.c ./sys/kern/uipc_socket.c
--- ../../freebsd-current/src/sys/kern/uipc_socket.c	2004-06-14 11:16:19.000000000 -0700
+++ ./sys/kern/uipc_socket.c	2004-06-16 15:37:32.000000000 -0700
@@ -173,25 +173,32 @@
 	int error;
 
 	if (proto)
-		prp = pffindproto(dom, proto, type);
+		prp = pfgetproto(dom, proto, type);
 	else
-		prp = pffindtype(dom, type);
+		prp = pfgettype(dom, type);
 
-	if (prp == NULL || prp->pr_usrreqs->pru_attach == NULL)
-		return (EPROTONOSUPPORT);
+	if (prp == NULL || prp->pr_usrreqs->pru_attach == NULL) {
+		error = EPROTONOSUPPORT;
+		goto out;
+	}
 
 	if (jailed(cred) && jail_socket_unixiproute_only &&
 	    prp->pr_domain->dom_family != PF_LOCAL &&
 	    prp->pr_domain->dom_family != PF_INET &&
 	    prp->pr_domain->dom_family != PF_ROUTE) {
-		return (EPROTONOSUPPORT);
+		error = EPROTONOSUPPORT;
+		goto out;
 	}
 
-	if (prp->pr_type != type)
-		return (EPROTOTYPE);
+	if (prp->pr_type != type) {
+		error = EPROTOTYPE;
+		goto out;
+	}
 	so = soalloc(M_WAITOK);
-	if (so == NULL)
-		return (ENOBUFS);
+	if (so == NULL) {
+		error = ENOBUFS;
+		goto out;
+	}
 
 	TAILQ_INIT(&so->so_incomp);
 	TAILQ_INIT(&so->so_comp);
@@ -209,10 +216,13 @@
 		SOCK_LOCK(so);
 		so->so_state |= SS_NOFDREF;
 		sorele(so);
-		return (error);
 	}
-	*aso = so;
-	return (0);
+out:
+	if (error && prp != NULL) 
+		pfput(prp);
+	else if (error == 0)	
+		*aso = so;
+	return (error);
 }
 
 int
@@ -414,6 +424,8 @@
 		int error2 = (*so->so_proto->pr_usrreqs->pru_detach)(so);
 		if (error == 0)
 			error = error2;
+		if (error2 == 0)
+			pfput(so->so_proto);
 	}
 discard:
 	SOCK_LOCK(so);
diff -ru ../../freebsd-current/src/sys/netinet/ip_input.c ./sys/netinet/ip_input.c
--- ../../freebsd-current/src/sys/netinet/ip_input.c	2004-06-13 10:29:09.000000000 -0700
+++ ./sys/netinet/ip_input.c	2004-06-16 15:30:42.000000000 -0700
@@ -254,7 +254,7 @@
 
 	TAILQ_INIT(&in_ifaddrhead);
 	in_ifaddrhashtbl = hashinit(INADDR_NHASH, M_IFADDR, &in_ifaddrhmask);
-	pr = pffindproto(PF_INET, IPPROTO_RAW, SOCK_RAW);
+	pr = pfgetproto(PF_INET, IPPROTO_RAW, SOCK_RAW);
 	if (pr == 0)
 		panic("ip_init");
 	for (i = 0; i < IPPROTO_MAX; i++)
diff -ru ../../freebsd-current/src/sys/netinet6/ip6_input.c ./sys/netinet6/ip6_input.c
--- ../../freebsd-current/src/sys/netinet6/ip6_input.c	2004-06-13 10:29:10.000000000 -0700
+++ ./sys/netinet6/ip6_input.c	2004-06-16 15:30:42.000000000 -0700
@@ -173,7 +173,7 @@
 	if (sizeof(struct protosw) != sizeof(struct ip6protosw))
 		panic("sizeof(protosw) != sizeof(ip6protosw)");
 #endif
-	pr = (struct ip6protosw *)pffindproto(PF_INET6, IPPROTO_RAW, SOCK_RAW);
+	pr = (struct ip6protosw *)pfgetproto(PF_INET6, IPPROTO_RAW, SOCK_RAW);
 	if (pr == 0)
 		panic("ip6_init");
 	for (i = 0; i < IPPROTO_MAX; i++)
diff -ru ../../freebsd-current/src/sys/sys/domain.h ./sys/sys/domain.h
--- ../../freebsd-current/src/sys/sys/domain.h	2004-04-06 21:19:49.000000000 -0700
+++ ./sys/sys/domain.h	2004-06-16 15:42:18.000000000 -0700
@@ -61,12 +61,14 @@
 	void	*(*dom_ifattach)(struct ifnet *);
 	oid	(*dom_ifdetach)(struct ifnet *, void *);
 					/* af-dependent data on ifnet */
+	u_long  dom_refcnt;		/* sockets ref domains */
 };
 
 #ifdef _KERNEL
 extern struct	domain *domains;
 extern struct	domain localdomain;
 extern void	net_add_domain(void *);
+extern int	net_remove_domain(struct domain *);
 
 #define DOMAIN_SET(name) \
 	SYSINIT(domain_ ## name, SI_SUB_PROTO_DOMAIN, SI_ORDER_SECOND, net_add_domain, & name ## domain)
diff -ru ../../freebsd-current/src/sys/sys/protosw.h ./sys/sys/protosw.h
--- ../../freebsd-current/src/sys/sys/protosw.h	2004-04-06 21:19:49.000000000 -0700
+++ ./sys/sys/protosw.h	2004-06-16 15:30:42.000000000 -0700
@@ -317,8 +317,9 @@
 #ifdef _KERNEL
 void	pfctlinput(int, struct sockaddr *);
 void	pfctlinput2(int, struct sockaddr *, void *);
-struct protosw *pffindproto(int family, int protocol, int type);
-struct protosw *pffindtype(int family, int type);
+struct protosw *pfgetproto(int family, int protocol, int type);
+struct protosw *pfgettype(int family, int type);
+void pfput(struct protosw *);
 #endif
 
 #endif
>Release-Note:
>Audit-Trail:
>Unformatted:


More information about the freebsd-bugs mailing list