From 7be2770a426dc7399b63115b1c2c7d8bebdea06a Mon Sep 17 00:00:00 2001
From: Mark Johnston <markj@FreeBSD.org>
Date: Thu, 3 Dec 2020 17:10:00 +0000
Subject: [PATCH] sdt: Create providers and probes in separate passes when
 loading sdt.ko

The sdt module's load handler iterates over SDT linker sets for the
kernel and all loaded modules to create probes and providers defined by
SDT(9).  Probes in one module may belong to a provider in a different
module, but when a probe is created we assume that the provider is
already defined.  To maintain this invariant, modify the load handler to
perform two separate passes over loaded modules: one to define providers
and the other to define probes.

The problem manifests when loading linux.ko, which depends on
linux_common.ko, which defines providers used by probes defined in
linux.ko.

Reported by:	gallatin
MFC after:	2 weeks
Sponsored by:	The FreeBSD Foundation
---
 sys/cddl/dev/sdt/sdt.c | 56 +++++++++++++++++++++++++++++-------------
 1 file changed, 39 insertions(+), 17 deletions(-)

diff --git a/sys/cddl/dev/sdt/sdt.c b/sys/cddl/dev/sdt/sdt.c
index c5f1d5619761..d352ed177e8c 100644
--- a/sys/cddl/dev/sdt/sdt.c
+++ b/sys/cddl/dev/sdt/sdt.c
@@ -272,25 +272,23 @@ sdt_destroy(void *arg, dtrace_id_t id, void *parg)
 {
 }
 
-/*
- * Called from the kernel linker when a module is loaded, before
- * dtrace_module_loaded() is called. This is done so that it's possible to
- * register new providers when modules are loaded. The DTrace framework
- * explicitly disallows calling into the framework from the provide_module
- * provider method, so we cannot do this there.
- */
 static void
-sdt_kld_load(void *arg __unused, struct linker_file *lf)
+sdt_kld_load_providers(struct linker_file *lf)
 {
 	struct sdt_provider **prov, **begin, **end;
-	struct sdt_probe **probe, **p_begin, **p_end;
-	struct sdt_argtype **argtype, **a_begin, **a_end;
 
 	if (linker_file_lookup_set(lf, "sdt_providers_set", &begin, &end,
 	    NULL) == 0) {
 		for (prov = begin; prov < end; prov++)
 			sdt_create_provider(*prov);
 	}
+}
+
+static void
+sdt_kld_load_probes(struct linker_file *lf)
+{
+	struct sdt_probe **probe, **p_begin, **p_end;
+	struct sdt_argtype **argtype, **a_begin, **a_end;
 
 	if (linker_file_lookup_set(lf, "sdt_probes_set", &p_begin, &p_end,
 	    NULL) == 0) {
@@ -311,6 +309,20 @@ sdt_kld_load(void *arg __unused, struct linker_file *lf)
 	}
 }
 
+/*
+ * Called from the kernel linker when a module is loaded, before
+ * dtrace_module_loaded() is called. This is done so that it's possible to
+ * register new providers when modules are loaded. The DTrace framework
+ * explicitly disallows calling into the framework from the provide_module
+ * provider method, so we cannot do this there.
+ */
+static void
+sdt_kld_load(void *arg __unused, struct linker_file *lf)
+{
+	sdt_kld_load_providers(lf);
+	sdt_kld_load_probes(lf);
+}
+
 static void
 sdt_kld_unload_try(void *arg __unused, struct linker_file *lf, int *error)
 {
@@ -349,16 +361,21 @@ sdt_kld_unload_try(void *arg __unused, struct linker_file *lf, int *error)
 }
 
 static int
-sdt_linker_file_cb(linker_file_t lf, void *arg __unused)
+sdt_load_providers_cb(linker_file_t lf, void *arg __unused)
 {
+	sdt_kld_load_providers(lf);
+	return (0);
+}
 
-	sdt_kld_load(NULL, lf);
-
+static int
+sdt_load_probes_cb(linker_file_t lf, void *arg __unused)
+{
+	sdt_kld_load_probes(lf);
 	return (0);
 }
 
 static void
-sdt_load()
+sdt_load(void)
 {
 
 	TAILQ_INIT(&sdt_prov_list);
@@ -370,12 +387,17 @@ sdt_load()
 	sdt_kld_unload_try_tag = EVENTHANDLER_REGISTER(kld_unload_try,
 	    sdt_kld_unload_try, NULL, EVENTHANDLER_PRI_ANY);
 
-	/* Pick up probes from the kernel and already-loaded linker files. */
-	linker_file_foreach(sdt_linker_file_cb, NULL);
+	/*
+	 * Pick up probes from the kernel and already-loaded linker files.
+	 * Define providers in a separate pass since a linker file may be using
+	 * providers defined in a file that appears later in the list.
+	 */
+	linker_file_foreach(sdt_load_providers_cb, NULL);
+	linker_file_foreach(sdt_load_probes_cb, NULL);
 }
 
 static int
-sdt_unload()
+sdt_unload(void)
 {
 	struct sdt_provider *prov, *tmp;
 	int ret;