git: 2e57144df7e1 - stable/13 - stand: module: unlink the entire tail when dependencies fail to load

From: Kyle Evans <kevans_at_FreeBSD.org>
Date: Sun, 21 Jul 2024 05:25:22 UTC
The branch stable/13 has been updated by kevans:

URL: https://cgit.FreeBSD.org/src/commit/?id=2e57144df7e1f8d9ed91a75f96ff2b8affc1c601

commit 2e57144df7e1f8d9ed91a75f96ff2b8affc1c601
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2024-06-25 20:31:50 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2024-07-21 05:25:07 +0000

    stand: module: unlink the entire tail when dependencies fail to load
    
    Assume you have loader configured to load linux64, which has a
    dependency on both linux_common and mqueuefs but neither the kernel
    nor kernel config in question have the mqueuefs module included.
    
    When the load command for linux64 fails to find mqueuefs, it will
    free both linux64 and linux_common as they were loaded first, but only
    linux64 gets removed from the module list.  As a result, future
    traversals hit an easy use-after-free with linux_common.
    
    Fix it so that we unlink the entire tail of the list.  Anything after
    the initially loaded module is, by definition, a dependency on the
    loaded module while we're still in the load command, so we can just
    discard the entire tail.  If linux_common were loaded before linux64, it
    should not move to a position during this load where it would suddenly
    be missing from the view presented to the kernel.
    
    Reported by:    philip
    Reviewed by:    imp, philip, tsoome
    
    (cherry picked from commit 3da568710fde08251996c117b87bedb326dedb57)
---
 stand/common/module.c | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/stand/common/module.c b/stand/common/module.c
index b4a53701ffba..699be7bf4feb 100644
--- a/stand/common/module.c
+++ b/stand/common/module.c
@@ -65,6 +65,7 @@ static char			*mod_searchmodule(char *name, struct mod_depend *verinfo);
 static char *			mod_searchmodule_pnpinfo(const char *bus, const char *pnpinfo);
 static void			file_insert_tail(struct preloaded_file *mp);
 static void			file_remove(struct preloaded_file *fp);
+static void			file_remove_tail(struct preloaded_file *fp);
 struct file_metadata*		metadata_next(struct file_metadata *base_mp, int type);
 static void			moduledir_readhints(struct moduledir *mdp);
 static void			moduledir_rebuild(void);
@@ -958,7 +959,7 @@ mod_loadkld(const char *kldname, int argc, char *argv[])
 		file_insert_tail(fp);	/* Add to the list of loaded files */
 		if (file_load_dependencies(fp) != 0) {
 			err = ENOENT;
-			file_remove(fp);
+			file_remove_tail(fp);
 			loadaddr = loadaddr_saved;
 			fp = NULL;
 			break;
@@ -1719,25 +1720,45 @@ file_insert_tail(struct preloaded_file *fp)
  * Remove module from the chain
  */
 static void
-file_remove(struct preloaded_file *fp)
+file_remove_impl(struct preloaded_file *fp, bool keep_tail)
 {
-	struct preloaded_file   *cm;
+	struct preloaded_file   *cm, *next;
 
 	if (preloaded_files == NULL)
 		return;
 
+	if (keep_tail)
+		next = fp->f_next;
+	else
+		next = NULL;
+
 	if (preloaded_files == fp) {
-		preloaded_files = fp->f_next;
+		preloaded_files = next;
 		return;
         }
+
         for (cm = preloaded_files; cm->f_next != NULL; cm = cm->f_next) {
 		if (cm->f_next == fp) {
-			cm->f_next = fp->f_next;
+			cm->f_next = next;
 			return;
 		}
 	}
 }
 
+static void
+file_remove(struct preloaded_file *fp)
+{
+
+	file_remove_impl(fp, true);
+}
+
+static void
+file_remove_tail(struct preloaded_file *fp)
+{
+
+	file_remove_impl(fp, false);
+}
+
 static char *
 moduledir_fullpath(struct moduledir *mdp, const char *fname)
 {