⬢ DragonFlyBSD Kernel Audit
DF-2233 / fix.diff
← back to finding ↓ download raw
--- a/sys/libiconv/iconv.c
+++ b/sys/libiconv/iconv.c
@@ -47,6 +47,7 @@
 MODULE_VERSION(libiconv, 2);
 
 static struct lock iconv_lock;
+static unsigned iconv_refs;	/* in-flight callers (unload drain guard) */
 
 #ifdef notnow
 /*
@@ -82,7 +83,13 @@
 {
 	struct iconv_cspair *csp;
 
+	if (iconv_refs)
+		return EBUSY;
 	lockmgr(&iconv_lock, LK_EXCLUSIVE);
+	if (iconv_refs) {
+		lockmgr(&iconv_lock, LK_RELEASE);
+		return EBUSY;
+	}
 	TAILQ_FOREACH(csp, &iconv_cslist, cp_link) {
 		if (csp->cp_refcount) {
 			lockmgr(&iconv_lock, LK_RELEASE);
@@ -93,6 +100,14 @@
 	while ((csp = TAILQ_FIRST(&iconv_cslist)) != NULL)
 		iconv_unregister_cspair(csp);
 	lockmgr(&iconv_lock, LK_RELEASE);
+	/*
+	 * Drain guard: callers take iconv_refs BEFORE acquiring the lock and
+	 * drop it after release, so refs==0 here guarantees nobody is
+	 * executing inside this module any more.  If someone raced in,
+	 * refuse the unload (kldunload returns EBUSY; retry).
+	 */
+	if (iconv_refs)
+		return EBUSY;
 	lockuninit(&iconv_lock);
 	return 0;
 }
@@ -143,7 +158,7 @@
 }
 
 static int
-iconv_lookupconv(const char *name, struct iconv_converter_class **dcpp)
+iconv_lookupconv_locked(const char *name, struct iconv_converter_class **dcpp)
 {
 	struct iconv_converter_class *dcp;
 
@@ -160,7 +175,8 @@
 }
 
 static int
-iconv_lookupcs(const char *to, const char *from, struct iconv_cspair **cspp)
+iconv_lookupcs_locked(const char *to, const char *from,
+	struct iconv_cspair **cspp)
 {
 	struct iconv_cspair *csp;
 
@@ -184,7 +200,7 @@
 	char *cp;
 	int csize, ucsto, ucsfrom;
 
-	if (iconv_lookupcs(to, from, NULL) == 0)
+	if (iconv_lookupcs_locked(to, from, NULL) == 0)
 		return EEXIST;
 	csize = sizeof(*csp);
 	ucsto = strcmp(to, iconv_unicode_string) == 0;
@@ -237,12 +253,18 @@
 	const char *cnvname;
 	int error;
 
+	atomic_add_int(&iconv_refs, 1);
 	/*
 	 * First, lookup fully qualified cspairs
 	 */
-	error = iconv_lookupcs(to, from, &csp);
-	if (error == 0)
-		return ICONV_CONVERTER_OPEN(csp->cp_dcp, csp, NULL, handle);
+	lockmgr(&iconv_lock, LK_SHARED);
+	error = iconv_lookupcs_locked(to, from, &csp);
+	if (error == 0) {
+		error = ICONV_CONVERTER_OPEN(csp->cp_dcp, csp, NULL, handle);
+		lockmgr(&iconv_lock, LK_RELEASE);
+		atomic_sub_int(&iconv_refs, 1);
+		return error;
+	}
 
 	/*
 	 * Well, nothing found. Now try to construct a composite conversion
@@ -252,17 +274,22 @@
 		cnvname = ICONV_CONVERTER_NAME(dcp);
 		if (cnvname == NULL)
 			continue;
-		error = iconv_lookupcs(cnvname, from, &cspfrom);
+		error = iconv_lookupcs_locked(cnvname, from, &cspfrom);
 		if (error)
 			continue;
-		error = iconv_lookupcs(to, cnvname, &cspto);
+		error = iconv_lookupcs_locked(to, cnvname, &cspto);
 		if (error)
 			continue;
 		/*
 		 * Fine, we're found a pair which can be combined together
 		 */
-		return ICONV_CONVERTER_OPEN(dcp, cspto, cspfrom, handle);
+		error = ICONV_CONVERTER_OPEN(dcp, cspto, cspfrom, handle);
+		lockmgr(&iconv_lock, LK_RELEASE);
+		atomic_sub_int(&iconv_refs, 1);
+		return error;
 	}
+	lockmgr(&iconv_lock, LK_RELEASE);
+	atomic_sub_int(&iconv_refs, 1);
 	return ENOENT;
 }
 
@@ -325,6 +352,7 @@
 	int error;
 
 	error = 0;
+	atomic_add_int(&iconv_refs, 1);
 
 	lockmgr(&iconv_lock, LK_SHARED);
 	TAILQ_FOREACH(dcp, &iconv_converters, cc_link) {
@@ -336,6 +364,7 @@
 			break;
 	}
 	lockmgr(&iconv_lock, LK_RELEASE);
+	atomic_sub_int(&iconv_refs, 1);
 	if (error)
 		return error;
 	spc = 0;
@@ -360,6 +389,7 @@
 	bzero(&csi, sizeof(csi));
 	csi.cs_version = ICONV_CSPAIR_INFO_VER;
 
+	atomic_add_int(&iconv_refs, 1);
 	lockmgr(&iconv_lock, LK_SHARED);
 	TAILQ_FOREACH(csp, &iconv_cslist, cp_link) {
 		csi.cs_id = csp->cp_id;
@@ -372,6 +402,7 @@
 			break;
 	}
 	lockmgr(&iconv_lock, LK_RELEASE);
+	atomic_sub_int(&iconv_refs, 1);
 	return error;
 }
 
@@ -383,11 +414,16 @@
 {
 	struct iconv_converter_class *dcp;
 	struct iconv_cspair *csp;
+	int error;
 
-	if (iconv_lookupconv(converter, &dcp) != 0)
+	lockmgr(&iconv_lock, LK_EXCLUSIVE);
+	if (iconv_lookupconv_locked(converter, &dcp) != 0) {
+		lockmgr(&iconv_lock, LK_RELEASE);
 		return EINVAL;
-
-	return iconv_register_cspair(to, from, dcp, NULL, &csp);
+	}
+	error = iconv_register_cspair(to, from, dcp, NULL, &csp);
+	lockmgr(&iconv_lock, LK_RELEASE);
+	return error;
 }
 
 /*
@@ -415,12 +451,17 @@
 		return EINVAL;
 	if (strlen(din.ia_converter) >= ICONV_CNVNMAXLEN)
 		return EINVAL;
-	if (iconv_lookupconv(din.ia_converter, &dcp) != 0)
-		return EINVAL;
+	atomic_add_int(&iconv_refs, 1);
 	lockmgr(&iconv_lock, LK_EXCLUSIVE);
+	if (iconv_lookupconv_locked(din.ia_converter, &dcp) != 0) {
+		lockmgr(&iconv_lock, LK_RELEASE);
+		atomic_sub_int(&iconv_refs, 1);
+		return EINVAL;
+	}
 	error = iconv_register_cspair(din.ia_to, din.ia_from, dcp, NULL, &csp);
 	if (error) {
 		lockmgr(&iconv_lock, LK_RELEASE);
+		atomic_sub_int(&iconv_refs, 1);
 		return error;
 	}
 	if (din.ia_datalen) {
@@ -434,11 +475,13 @@
 	if (error)
 		goto bad;
 	lockmgr(&iconv_lock, LK_RELEASE);
+	atomic_sub_int(&iconv_refs, 1);
 	ICDEBUG("%s => %s, %d bytes\n",din.ia_from, din.ia_to, din.ia_datalen);
 	return 0;
 bad:
 	iconv_unregister_cspair(csp);
 	lockmgr(&iconv_lock, LK_RELEASE);
+	atomic_sub_int(&iconv_refs, 1);
 	return error;
 }