Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 16 Aug 2021 06:08:20 GMT
From:      Kyle Evans <kevans@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 29e400e994ea - main - domain: make it safer to add domains post-domainfinalize
Message-ID:  <202108160608.17G68KXm029131@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by kevans:

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

commit 29e400e994ea19cb90736ceeb67dff716671ff25
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2020-06-26 02:38:27 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2021-08-16 05:59:56 +0000

    domain: make it safer to add domains post-domainfinalize
    
    I can see two concerns for adding domains after domainfinalize:
    
    1.) The slow/fast callouts have already been setup.
    2.) Userland could create a socket while we're in the middle of
      initialization.
    
    We can address #1 fairly easily by tracking whether the domain's been
    initialized for at least the default vnet. There are still some concerns
    about the callbacks being invoked while a vnet is in the process of
    being created/destroyed, but this is a pre-existing issue that the
    callbacks must coordinate anyways.
    
    We should also address #2, but technically this has been an issue
    anyways because we don't assert on post-domainfinalize additions; we
    don't seem to hit it in practice.
    
    Future work can fix that up to make sure we don't find partially
    constructed domains, but care must be taken to make sure that at least,
    e.g., the usages of pffindproto in ip_input.c can still find them.
    
    Differential Revision:  https://reviews.freebsd.org/D25459
---
 sys/kern/uipc_domain.c | 27 ++++++++++++++++-----------
 sys/sys/domain.h       |  1 +
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/sys/kern/uipc_domain.c b/sys/kern/uipc_domain.c
index 50d6c0a9d9f8..b6aefec9556a 100644
--- a/sys/kern/uipc_domain.c
+++ b/sys/kern/uipc_domain.c
@@ -47,6 +47,8 @@ __FBSDID("$FreeBSD$");
 #include <sys/socketvar.h>
 #include <sys/systm.h>
 
+#include <machine/atomic.h>
+
 #include <net/vnet.h>
 
 /*
@@ -177,6 +179,8 @@ domain_init(void *arg)
 	flags = atomic_load_acq_int(&dp->dom_flags);
 	if ((flags & DOMF_SUPPORTED) == 0)
 		return;
+	KASSERT((flags & DOMF_INITED) == 0 || !IS_DEFAULT_VNET(curvnet),
+	    ("Premature initialization of domain in non-default vnet"));
 	if (dp->dom_init)
 		(*dp->dom_init)();
 	for (pr = dp->dom_protosw; pr < dp->dom_protoswNPROTOSW; pr++)
@@ -188,6 +192,8 @@ domain_init(void *arg)
 	max_datalen = MHLEN - max_hdr;
 	if (max_datalen < 1)
 		panic("%s: max_datalen < 1", __func__);
+	if (IS_DEFAULT_VNET(curvnet))
+		atomic_set_rel_int(&dp->dom_flags, DOMF_INITED);
 }
 
 #ifdef VIMAGE
@@ -236,15 +242,6 @@ domain_add(void *data)
 	if (domain_init_status < 1)
 		printf("WARNING: attempt to domain_add(%s) before "
 		    "domaininit()\n", dp->dom_name);
-#endif
-#ifdef notyet
-	KASSERT(domain_init_status < 2,
-	    ("attempt to domain_add(%s) after domainfinalize()",
-	    dp->dom_name));
-#else
-	if (domain_init_status >= 2)
-		printf("WARNING: attempt to domain_add(%s) after "
-		    "domainfinalize()\n", dp->dom_name);
 #endif
 	mtx_unlock(&dom_mtx);
 }
@@ -491,10 +488,14 @@ pfslowtimo(void *arg)
 	struct protosw *pr;
 
 	NET_EPOCH_ENTER(et);
-	for (dp = domains; dp; dp = dp->dom_next)
+	for (dp = domains; dp; dp = dp->dom_next) {
+		if ((atomic_load_int(&dp->dom_flags) & DOMF_INITED) == 0)
+			continue;
+		atomic_thread_fence_acq();
 		for (pr = dp->dom_protosw; pr < dp->dom_protoswNPROTOSW; pr++)
 			if (pr->pr_slowtimo)
 				(*pr->pr_slowtimo)();
+	}
 	NET_EPOCH_EXIT(et);
 	callout_reset(&pfslow_callout, hz/2, pfslowtimo, NULL);
 }
@@ -507,10 +508,14 @@ pffasttimo(void *arg)
 	struct protosw *pr;
 
 	NET_EPOCH_ENTER(et);
-	for (dp = domains; dp; dp = dp->dom_next)
+	for (dp = domains; dp; dp = dp->dom_next) {
+		if ((atomic_load_int(&dp->dom_flags) & DOMF_INITED) == 0)
+			continue;
+		atomic_thread_fence_acq();
 		for (pr = dp->dom_protosw; pr < dp->dom_protoswNPROTOSW; pr++)
 			if (pr->pr_fasttimo)
 				(*pr->pr_fasttimo)();
+	}
 	NET_EPOCH_EXIT(et);
 	callout_reset(&pffast_callout, hz/5, pffasttimo, NULL);
 }
diff --git a/sys/sys/domain.h b/sys/sys/domain.h
index a59f4e230a1d..dc09c9786443 100644
--- a/sys/sys/domain.h
+++ b/sys/sys/domain.h
@@ -74,6 +74,7 @@ struct domain {
 
 /* dom_flags */
 #define	DOMF_SUPPORTED	0x0001	/* System supports this domain. */
+#define	DOMF_INITED	0x0002	/* Initialized in the default vnet. */
 
 #ifdef _KERNEL
 extern int	domain_init_status;



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202108160608.17G68KXm029131>