Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 29 Jun 2020 21:35:50 +0000 (UTC)
From:      Andrew Gallatin <gallatin@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r362789 - head/sys/kern
Message-ID:  <202006292135.05TLZo2d034398@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: gallatin
Date: Mon Jun 29 21:35:50 2020
New Revision: 362789
URL: https://svnweb.freebsd.org/changeset/base/362789

Log:
  Fix a panic when unloading firmware
  
  LIST_FOREACH_SAFE() is not safe in the presence
  of other threads removing list entries when a
  mutex is released.
  
  This is not in the critical path, so just restart
  the scan each time we drop the lock, rather than
  using a marker.
  
  Reviewed by:	jhb, markj
  Sponsored by:	Netflix

Modified:
  head/sys/kern/subr_firmware.c

Modified: head/sys/kern/subr_firmware.c
==============================================================================
--- head/sys/kern/subr_firmware.c	Mon Jun 29 19:30:35 2020	(r362788)
+++ head/sys/kern/subr_firmware.c	Mon Jun 29 21:35:50 2020	(r362789)
@@ -394,14 +394,12 @@ EVENTHANDLER_DEFINE(mountroot, firmware_mountroot, NUL
 static void
 unloadentry(void *unused1, int unused2)
 {
-	struct priv_fw *fp, *tmp;
+	struct priv_fw *fp;
 	int err;
-	bool changed;
 
 	mtx_lock(&firmware_mtx);
-	changed = false;
 restart:
-	LIST_FOREACH_SAFE(fp, &firmware_table, link, tmp) {
+	LIST_FOREACH(fp, &firmware_table, link) {
 		if (fp->file == NULL || fp->refcnt != 0 ||
 		    (fp->flags & FW_UNLOAD) == 0)
 			continue;
@@ -412,7 +410,6 @@ restart:
 		 * 2. clear FW_UNLOAD so we don't try this entry again.
 		 * 3. release the lock while trying to unload the module.
 		 */
-		changed = true;
 		fp->flags &= ~FW_UNLOAD;	/* do not try again */
 
 		/*
@@ -422,9 +419,11 @@ restart:
 		mtx_unlock(&firmware_mtx);
 		err = linker_release_module(NULL, NULL, fp->file);
 		mtx_lock(&firmware_mtx);
-	}
-	if (changed) {
-		changed = false;
+
+		/*
+		 * When we dropped the lock, another thread could have
+		 * removed an element, so we must restart the scan.
+		 */
 		goto restart;
 	}
 	mtx_unlock(&firmware_mtx);



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