git: 7ef5c19b219e - main - kern linker: Don't invoke dtors without having invoked ctors

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Sun, 31 Mar 2024 18:15:53 UTC
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=7ef5c19b219e47684afd9d8d9126df39edc8d885

commit 7ef5c19b219e47684afd9d8d9126df39edc8d885
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2024-03-31 18:14:02 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2024-03-31 18:15:11 +0000

    kern linker: Don't invoke dtors without having invoked ctors
    
    I have a kernel module which fails to load because of an unrecognized
    relocation type.  link_elf_load_file() fails before the module's ctors
    are invoked and it calls linker_file_unload(), which causes the module's
    dtors to be executed, resulting in a kernel panic.
    
    Add a flag to the linker file to ensure that dtors are not invoked if
    unloading due to an error prior to ctors being invoked.
    
    At the moment I only implemented this for link_elf_obj.c since
    link_elf.c doesn't invoke dtors, but I refactored link_elf.c to make
    them more similar.
    
    Fixes:          9e575fadf491 ("link_elf_obj: Invoke fini callbacks")
    Reviewed by:    zlei, kib
    MFC after:      2 weeks
    Differential Revision:  https://reviews.freebsd.org/D44559
---
 sys/kern/kern_linker.c  |  1 +
 sys/kern/link_elf.c     | 15 +++++++++++++--
 sys/kern/link_elf_obj.c | 30 +++++++++++++++++++++++++++---
 sys/sys/linker.h        |  5 +++++
 4 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/sys/kern/kern_linker.c b/sys/kern/kern_linker.c
index 54b7466124db..08b85867d781 100644
--- a/sys/kern/kern_linker.c
+++ b/sys/kern/kern_linker.c
@@ -657,6 +657,7 @@ linker_make_file(const char *pathname, linker_class_t lc)
 		return (NULL);
 	lf->ctors_addr = 0;
 	lf->ctors_size = 0;
+	lf->ctors_invoked = LF_NONE;
 	lf->dtors_addr = 0;
 	lf->dtors_size = 0;
 	lf->refs = 1;
diff --git a/sys/kern/link_elf.c b/sys/kern/link_elf.c
index b08c19f3c018..9f9c10456b60 100644
--- a/sys/kern/link_elf.c
+++ b/sys/kern/link_elf.c
@@ -358,7 +358,7 @@ link_elf_error(const char *filename, const char *s)
 }
 
 static void
-link_elf_invoke_ctors(caddr_t addr, size_t size)
+link_elf_invoke_cbs(caddr_t addr, size_t size)
 {
 	void (**ctor)(void);
 	size_t i, cnt;
@@ -373,6 +373,17 @@ link_elf_invoke_ctors(caddr_t addr, size_t size)
 	}
 }
 
+static void
+link_elf_invoke_ctors(linker_file_t lf)
+{
+	KASSERT(lf->ctors_invoked == LF_NONE,
+	    ("%s: file %s ctor state %d",
+	    __func__, lf->filename, lf->ctors_invoked));
+
+	link_elf_invoke_cbs(lf->ctors_addr, lf->ctors_size);
+	lf->ctors_invoked = LF_CTORS;
+}
+
 /*
  * Actions performed after linking/loading both the preloaded kernel and any
  * modules; whether preloaded or dynamicly loaded.
@@ -403,7 +414,7 @@ link_elf_link_common_finish(linker_file_t lf)
 #endif
 
 	/* Invoke .ctors */
-	link_elf_invoke_ctors(lf->ctors_addr, lf->ctors_size);
+	link_elf_invoke_ctors(lf);
 	return (0);
 }
 
diff --git a/sys/kern/link_elf_obj.c b/sys/kern/link_elf_obj.c
index c8ccebea0832..a7c7d4826322 100644
--- a/sys/kern/link_elf_obj.c
+++ b/sys/kern/link_elf_obj.c
@@ -658,6 +658,30 @@ link_elf_invoke_cbs(caddr_t addr, size_t size)
 	}
 }
 
+static void
+link_elf_invoke_ctors(linker_file_t lf)
+{
+	KASSERT(lf->ctors_invoked == LF_NONE,
+	    ("%s: file %s ctor state %d",
+	    __func__, lf->filename, lf->ctors_invoked));
+
+	link_elf_invoke_cbs(lf->ctors_addr, lf->ctors_size);
+	lf->ctors_invoked = LF_CTORS;
+}
+
+static void
+link_elf_invoke_dtors(linker_file_t lf)
+{
+	KASSERT(lf->ctors_invoked != LF_DTORS,
+	    ("%s: file %s ctor state %d",
+	    __func__, lf->filename, lf->ctors_invoked));
+
+	if (lf->ctors_invoked == LF_CTORS) {
+		link_elf_invoke_cbs(lf->dtors_addr, lf->dtors_size);
+		lf->ctors_invoked = LF_DTORS;
+	}
+}
+
 static int
 link_elf_link_preload_finish(linker_file_t lf)
 {
@@ -684,7 +708,7 @@ link_elf_link_preload_finish(linker_file_t lf)
 	/* Apply protections now that relocation processing is complete. */
 	link_elf_protect(ef);
 
-	link_elf_invoke_cbs(lf->ctors_addr, lf->ctors_size);
+	link_elf_invoke_ctors(lf);
 	return (0);
 }
 
@@ -1239,7 +1263,7 @@ link_elf_load_file(linker_class_t cls, const char *filename,
 #endif
 
 	link_elf_protect(ef);
-	link_elf_invoke_cbs(lf->ctors_addr, lf->ctors_size);
+	link_elf_invoke_ctors(lf);
 	*result = lf;
 
 out:
@@ -1259,7 +1283,7 @@ link_elf_unload_file(linker_file_t file)
 	elf_file_t ef = (elf_file_t) file;
 	u_int i;
 
-	link_elf_invoke_cbs(file->dtors_addr, file->dtors_size);
+	link_elf_invoke_dtors(file);
 
 	/* Notify MD code that a module is being unloaded. */
 	elf_cpu_unload_file(file);
diff --git a/sys/sys/linker.h b/sys/sys/linker.h
index b6184f4fc876..52fbeb5584ff 100644
--- a/sys/sys/linker.h
+++ b/sys/sys/linker.h
@@ -82,6 +82,11 @@ struct linker_file {
     size_t		size;		/* size of file */
     caddr_t		ctors_addr;	/* address of .ctors/.init_array */
     size_t		ctors_size;	/* size of .ctors/.init_array */
+    enum {
+	    LF_NONE = 0,
+	    LF_CTORS,
+	    LF_DTORS,
+    } ctors_invoked;			/* have we run ctors yet? */
     caddr_t		dtors_addr;	/* address of .dtors/.fini_array */
     size_t		dtors_size;	/* size of .dtors/.fini_array */
     int			ndeps;		/* number of dependencies */