Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 7 Jun 2021 16:33:04 GMT
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 620a48f64d08 - stable/13 - kern linker: do not allow more than one kldload and kldunload syscalls simultaneously
Message-ID:  <202106071633.157GX4pR004097@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by kib:

URL: https://cgit.FreeBSD.org/src/commit/?id=620a48f64d0848ce4ef698b3c2aae960a146cc42

commit 620a48f64d0848ce4ef698b3c2aae960a146cc42
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2021-05-20 14:50:43 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2021-06-07 16:32:42 +0000

    kern linker: do not allow more than one kldload and kldunload syscalls simultaneously
    
    (cherry picked from commit e266a0f7f001c7886eab56d8c058d92d87010400)
---
 sys/kern/kern_linker.c | 86 +++++++++++++++++++++++++++++++++++++++-----------
 sys/sys/linker.h       |  9 ++++++
 2 files changed, 77 insertions(+), 18 deletions(-)

diff --git a/sys/kern/kern_linker.c b/sys/kern/kern_linker.c
index e54546eec7b4..dbbf240a2f56 100644
--- a/sys/kern/kern_linker.c
+++ b/sys/kern/kern_linker.c
@@ -106,6 +106,7 @@ MALLOC_DEFINE(M_LINKER, "linker", "kernel linker");
 linker_file_t linker_kernel_file;
 
 static struct sx kld_sx;	/* kernel linker lock */
+static bool kld_busy;
 
 /*
  * Load counter used by clients to determine if a linker file has been
@@ -1050,6 +1051,49 @@ linker_search_symbol_name(caddr_t value, char *buf, u_int buflen,
 	    M_WAITOK));
 }
 
+int
+linker_kldload_busy(int flags)
+{
+	int error;
+
+	MPASS((flags & ~(LINKER_UB_UNLOCK | LINKER_UB_LOCKED |
+	    LINKER_UB_PCATCH)) == 0);
+	if ((flags & LINKER_UB_LOCKED) != 0)
+		sx_assert(&kld_sx, SA_XLOCKED);
+
+	if ((flags & LINKER_UB_LOCKED) == 0)
+		sx_xlock(&kld_sx);
+	while (kld_busy) {
+		error = sx_sleep(&kld_busy, &kld_sx,
+		    (flags & LINKER_UB_PCATCH) != 0 ? PCATCH : 0,
+		    "kldbusy", 0);
+		if (error != 0) {
+			if ((flags & LINKER_UB_UNLOCK) != 0)
+				sx_xunlock(&kld_sx);
+			return (error);
+		}
+	}
+	kld_busy = true;
+	if ((flags & LINKER_UB_UNLOCK) != 0)
+		sx_xunlock(&kld_sx);
+	return (0);
+}
+
+void
+linker_kldload_unbusy(int flags)
+{
+	MPASS((flags & ~LINKER_UB_LOCKED) == 0);
+	if ((flags & LINKER_UB_LOCKED) != 0)
+		sx_assert(&kld_sx, SA_XLOCKED);
+
+	if ((flags & LINKER_UB_LOCKED) == 0)
+		sx_xlock(&kld_sx);
+	MPASS(kld_busy);
+	kld_busy = false;
+	wakeup(&kld_busy);
+	sx_xunlock(&kld_sx);
+}
+
 /*
  * Syscalls.
  */
@@ -1066,12 +1110,6 @@ kern_kldload(struct thread *td, const char *file, int *fileid)
 	if ((error = priv_check(td, PRIV_KLD_LOAD)) != 0)
 		return (error);
 
-	/*
-	 * It is possible that kldloaded module will attach a new ifnet,
-	 * so vnet context must be set when this ocurs.
-	 */
-	CURVNET_SET(TD_TO_VNET(td));
-
 	/*
 	 * If file does not contain a qualified name or any dot in it
 	 * (kldname.ko, or kldname.ver.ko) treat it as an interface
@@ -1085,19 +1123,27 @@ kern_kldload(struct thread *td, const char *file, int *fileid)
 		modname = file;
 	}
 
-	sx_xlock(&kld_sx);
-	error = linker_load_module(kldname, modname, NULL, NULL, &lf);
-	if (error) {
+	error = linker_kldload_busy(LINKER_UB_PCATCH);
+	if (error != 0) {
 		sx_xunlock(&kld_sx);
-		goto done;
+		return (error);
 	}
-	lf->userrefs++;
-	if (fileid != NULL)
-		*fileid = lf->id;
-	sx_xunlock(&kld_sx);
 
-done:
+	/*
+	 * It is possible that kldloaded module will attach a new ifnet,
+	 * so vnet context must be set when this ocurs.
+	 */
+	CURVNET_SET(TD_TO_VNET(td));
+
+	error = linker_load_module(kldname, modname, NULL, NULL, &lf);
 	CURVNET_RESTORE();
+
+	if (error == 0) {
+		lf->userrefs++;
+		if (fileid != NULL)
+			*fileid = lf->id;
+	}
+	linker_kldload_unbusy(LINKER_UB_LOCKED);
 	return (error);
 }
 
@@ -1132,8 +1178,13 @@ kern_kldunload(struct thread *td, int fileid, int flags)
 	if ((error = priv_check(td, PRIV_KLD_UNLOAD)) != 0)
 		return (error);
 
+	error = linker_kldload_busy(LINKER_UB_PCATCH);
+	if (error != 0) {
+		sx_xunlock(&kld_sx);
+		return (error);
+	}
+
 	CURVNET_SET(TD_TO_VNET(td));
-	sx_xlock(&kld_sx);
 	lf = linker_find_file_by_id(fileid);
 	if (lf) {
 		KLD_DPF(FILE, ("kldunload: lf->userrefs=%d\n", lf->userrefs));
@@ -1153,9 +1204,8 @@ kern_kldunload(struct thread *td, int fileid, int flags)
 		}
 	} else
 		error = ENOENT;
-	sx_xunlock(&kld_sx);
-
 	CURVNET_RESTORE();
+	linker_kldload_unbusy(LINKER_UB_LOCKED);
 	return (error);
 }
 
diff --git a/sys/sys/linker.h b/sys/sys/linker.h
index 13274ecd1a7c..6423a3989a02 100644
--- a/sys/sys/linker.h
+++ b/sys/sys/linker.h
@@ -196,6 +196,15 @@ int linker_search_symbol_name(caddr_t value, char *buf, u_int buflen,
 /* HWPMC helper */
 void *linker_hwpmc_list_objects(void);
 
+/* kldload/kldunload syscalls blocking */
+#define	LINKER_UB_UNLOCK	0x0001	/* busy: unlock kld_sx locked on
+					   return */
+#define	LINKER_UB_LOCKED	0x0002	/* busy/unbusy: kld_sx locked on
+					   entry */
+#define	LINKER_UB_PCATCH	0x0004	/* busy: sleep interruptible */
+int linker_kldload_busy(int flags);
+void linker_kldload_unbusy(int flags);
+
 #endif	/* _KERNEL */
 
 /*



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