make module loading actually work, fix a couple of leaks, tie it into filesystem calls to load from device

Author: braindigitalis

.idea/inspectionProfiles/Project_Default.xml

@@ -118,6 +118,7 @@ foreach(mod ${MODULE_DIRS})
             add_custom_command(OUTPUT ${MOD_OUT}
                     COMMAND ${CMAKE_COMMAND} -E make_directory ${CMAKE_BINARY_DIR}/iso/system/modules
                     COMMAND ${CMAKE_LINKER} -r -o ${MOD_OUT} $<TARGET_OBJECTS:mod_${mod}>
+                    COMMAND ${CMAKE_COMMAND} -E touch ${MOD_OUT}
                     DEPENDS mod_${mod}
                     VERBATIM
             )

CMakeLists.txt

@@ -14,6 +14,16 @@
 #include <kernel.h>
 #include <stdbool.h>
 
+/**
+ * @brief module ABI version the kernel expects
+ *
+ * @note If we change the ABI at all, we must bump this by 1.
+ * It is used in the names of the init/exit functions of each module,
+ * so an incompatible module is outright rejected before it can wreak
+ * havoc in the kernel space.
+ */
+#define KMOD_ABI 100
+
 /* ELF identification */
 #define EI_NIDENT      16   /**< Size of e_ident[] array */
 
@@ -127,8 +137,29 @@ typedef struct elf_rela {
 typedef bool (*module_init_fn)(void);  /**< Prototype for module initialiser */
 typedef void (*module_exit_fn)(void);  /**< Prototype for module finaliser */
 
+/**
+ * @brief Set this prefix on all module_init/module_exit functions in modules
+ */
 #define EXPORTED __attribute__((visibility("default")))
 
+/**
+ * @brief helpers to form symbol names
+ */
+#define MOD_CAT2(a,b) a##b
+#define MOD_CAT(a,b)  MOD_CAT2(a,b)
+#define MOD_INIT_SYM(ver) MOD_CAT(mod_init_v, ver)
+#define MOD_EXIT_SYM(ver) MOD_CAT(mod_exit_v, ver)
+
+/* stringification */
+#define STR2(x) #x
+#define STR(x) STR2(x)
+
+/**
+ * @brief canonical names as strings
+ */
+#define MOD_INIT_NAME_STR "mod_init_v" STR(KMOD_ABI)
+#define MOD_EXIT_NAME_STR "mod_exit_v" STR(KMOD_ABI)
+
 /**
  * @brief Describes a loaded and relocated module image
  *
@@ -137,8 +168,9 @@ typedef void (*module_exit_fn)(void);  /**< Prototype for module finaliser */
  * lookup, and resolves conventional entry points named mod_init and mod_exit
  */
 typedef struct module {
-	char name[64];			/**< Optional friendly name, not set by the loader */
+	const char* name;		/**< Module name */
 	uint8_t *base;			/**< Base of contiguous allocation holding SHF_ALLOC sections */
+	void* raw_bits;			/**< For symbol resolution by other modules */
 	size_t size;			/**< Total size of the contiguous allocation */
 	const elf_sym *symtab;		/**< Pointer to the module's SHT_SYMTAB within the file buffer */
 	size_t sym_count;		/**< Number of entries in symtab */
@@ -156,9 +188,9 @@ typedef struct module {
  * @brief Load and relocate a module from an in-memory ELF64 ET_REL buffer, then call its initialiser
  *
  * Validates the ELF header, allocates a single contiguous region for all SHF_ALLOC sections,
- * copies/zeros contents with correct alignment, loads symtab/strtab, applies all SHT_RELA
- * relocations, resolves undefined globals against the kernel dynamic symbol table
- * (kernel must keep .dynsym/.dynstr), resolves mod_init/mod_exit, then calls mod_init
+ * copies/zeros contents with correct alignment, applies all SHT_RELA relocations, resolves
+ * undefined globals against the kernel dynamic symbol table (from kernel.sym),
+ * resolves versioned mod_init/mod_exit, then calls mod_init
  *
  * @param file pointer to the ELF bytes
  * @param len  size of the ELF buffer in bytes
@@ -178,7 +210,7 @@ bool module_load_from_memory(const void *file, size_t len, module *out);
  * @param m module descriptor previously returned by module_load_from_memory
  * @return true on success, false if @p m is NULL or teardown fails
  */
-bool module_unload(module *m);
+bool module_internal_unload(module *m);
 
 /**
  * @brief Resolve a kernel global by name via the kernel dynamic symbol table
@@ -297,3 +329,34 @@ uintptr_t module_resolve_symbol_addr(const module *m, const elf_sym *s);
  *          externals should appear as R_X86_64_64 when modules are built with -mcmodel=large -fno-pic -fno-plt
  */
 bool module_apply_relocations(const uint8_t *file, const elf_shdr *sh, size_t shnum, module *m);
+
+/**
+ * @brief Initialise the module registry and loader state
+ *
+ * Creates the in-kernel hashmap used to track loaded modules keyed by name
+ * Must be called once before any other module operations
+ */
+void init_modules(void);
+
+/**
+ * @brief Load and start a module by name
+ *
+ * Constructs the path /system/modules/<name>.ko, reads the file, relocates it,
+ * resolves externals against the kernel symbol index, and calls mod_init
+ * Returns false if the module is already loaded or on any failure
+ *
+ * @param name NUL-terminated module base name without extension or path
+ * @return true on success, false on error
+ */
+bool load_module(const char *name);
+
+/**
+ * @brief Stop and unload a loaded module by name
+ *
+ * Locates the module in the registry, calls mod_exit if present, frees all
+ * allocations associated with the image, and removes the entry from the registry
+ *
+ * @param name NUL-terminated module base name
+ * @return true on success, false if not found or unload failed
+ */
+bool unload_module(const char *name);

include/module.h

@@ -1,10 +1,10 @@
 #include <kernel.h>
 
-bool EXPORTED mod_init(void) {
+bool EXPORTED MOD_INIT_SYM(KMOD_ABI)(void) {
 	dprintf("test.ko: mod_init called!\n");
 	return true;
 }
 
-void EXPORTED mod_exit(void) {
+void EXPORTED MOD_EXIT_SYM(KMOD_ABI)(void) {
 	dprintf("test.ko: mod_exit called!\n");
 }

modules/test/test.c

@@ -8,25 +8,21 @@ const char* make_full_path(struct basic_ctx* ctx, const char* relative)
 {
 	uint8_t cpu = logical_cpu_id();
 	if (*relative == '/') {
-		dprintf("make_full_path %s -> %s\n", relative, relative);
 		return relative;
 	}
 
 	const char* csd = proc_cur(cpu)->csd;
 	char qualified_path[MAX_STRINGLEN];
 
 	if (*relative == 0) {
-		dprintf("make_full_path %s -> %s\n", relative, csd);
 		return csd;
 	}
 
 	if (*csd == '/' && *(csd+1) == 0) {
 		snprintf(qualified_path, MAX_STRINGLEN, "%s%s", csd, relative);
-		dprintf("make_full_path %s -> %s\n", relative, qualified_path);
 		return gc_strdup(ctx, qualified_path);
 	}
 	snprintf(qualified_path, MAX_STRINGLEN, "%s/%s", csd, relative);
-	dprintf("make_full_path %s -> %s\n", relative, qualified_path);
 	return gc_strdup(ctx, qualified_path);
 }
 

src/basic/file_io.c

@@ -69,7 +69,6 @@ fs_directory_entry_t* parse_fat32_directory(fs_tree_t* tree, fat32_t* info, uint
 				info->volume_name = strdup(dotless);
 				highest_lfn_order = -1;
 				memset(&lfns, 0, sizeof(lfns));
-				dprintf("volume id\n");
 			} else if ((entry->attr & ATTR_LONG_NAME) == ATTR_LONG_NAME) {
 				/* LFN entry: stash by sequence index (mask off 0x40) */
 				lfn_t* lfn = (lfn_t*)entry;
@@ -80,12 +79,10 @@ fs_directory_entry_t* parse_fat32_directory(fs_tree_t* tree, fat32_t* info, uint
 						highest_lfn_order = (int16_t)idx;
 					}
 				}
-				dprintf("long name\n");
 			} else {
 				/* SFN (regular file/dir) - build entry, optionally with LFN */
 				char sfn[13];
 				char sfn_dotless[13];
-				dprintf("short name\n");
 				parse_short_name(entry, sfn, sfn_dotless);
 
 				fs_directory_entry_t* file = kmalloc(sizeof(fs_directory_entry_t));
@@ -130,8 +127,6 @@ fs_directory_entry_t* parse_fat32_directory(fs_tree_t* tree, fat32_t* info, uint
 
 				file->next = list;
 				list = file;
-
-				dprintf("Append name\n");
 			}
 
 			next_entry:

src/fs/fat32/directory.c

@@ -243,7 +243,6 @@ bool make_vfs_path(const char* path, fs_tree_t* node) {
 		kfree_null(&pathinfo);
 		find.parent = pathname;
 		void* new_item = hashmap_set(vfs_hash, &find);
-		dprintf("VFS tree branch: %s\n", path);
 		/* If parent doesn't exist, create it */
 		bool parent_is_root = (strcmp(find.parent, "/") == 0);
 		if (!find_vfs_path(find.parent)) {
@@ -1004,7 +1003,6 @@ void retrieve_node_from_driver(fs_tree_t* node)
 	 * delete the old content first to avoid a memleak.
 	 */
 
-	dprintf("retrieve_node_from_driver\n");
 	if (node == NULL) {
 		return;
 	}
@@ -1142,11 +1140,9 @@ uint8_t verify_path(const char* path)
 fs_tree_t* walk_to_node(fs_tree_t* current_node, const char* path)
 {
 	if (!verify_path(path)) {
-		dprintf("walk_to_node: Verify path failed: %s\n", path);
 		return NULL;
 	}
 	if (!strcmp(path, "/")) {
-		dprintf("walk_to_node: returning root of tree: %s\n", path);
 		return fs_tree;
 	}
 	/* First build the dir stack */
@@ -1164,7 +1160,6 @@ fs_tree_t* walk_to_node(fs_tree_t* current_node, const char* path)
 		if (*parse == '/') {
 			*parse = 0;
 			walk->name = strdup(last);
-			dprintf("Dirstack part: '%s'\n", walk->name);
 			last = parse + 1;
 
                         dirstack_t* next = kmalloc(sizeof(dirstack_t));
@@ -1179,9 +1174,7 @@ fs_tree_t* walk_to_node(fs_tree_t* current_node, const char* path)
 	}
 	walk->next = NULL;
 	walk->name = strdup(last);
-	dprintf("Made dirstack: '%s'\n", path);
 	fs_tree_t* result = walk_to_node_internal(current_node, ds);
-	dprintf("Walked nodes, got result %p\n", result);
 	while (ds) {
 		dirstack_t* next = ds->next;
 		kfree_null(&ds->name);
@@ -1191,8 +1184,7 @@ fs_tree_t* walk_to_node(fs_tree_t* current_node, const char* path)
 	return result;
 }
 
-fs_directory_entry_t* find_file_in_dir(fs_tree_t* directory, const char* filename)
-{
+fs_directory_entry_t* find_file_in_dir(fs_tree_t* directory, const char* filename)  {
 	if (!directory || !filename) {
 		fs_set_error(FS_ERR_INVALID_ARG);
 		return NULL;
@@ -1511,7 +1503,6 @@ int attach_filesystem(const char* virtual_path, filesystem_t* fs, void* opaque)
 	}
 	item->responsible_driver = (void*)fs;
 	item->name = !strcmp(virtual_path, "/") ? strdup("/") : fs_get_name_part(virtual_path);
-	dprintf("Attach virtual path '%s' -> '%s'", virtual_path, item->name ? item->name : "<NULL>");
 	item->opaque = opaque;
 	item->dirty = 1;
 	item->files = NULL;

src/fs/filesystem.c

@@ -166,8 +166,6 @@ fs_directory_entry_t *parse_directory(fs_tree_t *node, iso9660 *info, uint32_t s
 				thisentry->filename[j] = 0;
 			}
 
-			dprintf("Retrieved filename: %s\n", thisentry->filename);
-
 			thisentry->year = fentry->recording_date.years_since_1900 + 1900;
 			thisentry->month = fentry->recording_date.month;
 			thisentry->day = fentry->recording_date.day;

src/fs/iso9660.c

@@ -10,7 +10,7 @@ init_func_t init_funcs[] = {
 	init_acpi, init_idt, boot_aps, init_pci, init_realtime_clock,
 	init_devicenames, init_keyboard, init_ide, init_ahci,
 	init_filesystem, init_devfs, init_iso9660, init_fat32,
-	init_rfs, init_rtl8139, init_e1000,
+	init_rfs, init_modules, init_rtl8139, init_e1000,
 	NULL,
 };
 
@@ -19,7 +19,7 @@ char* init_funcs_names[] = {
 	"idt",			"cpus",			"pci",		"clock",
 	"devicenames",		"keyboard",		"ide",		"ahci",
 	"filesystem",		"devfs",		"iso9660",	"fat32",
-	"rfs",			"rtl8139",		"e1000",
+	"rfs",			"modules",		"rtl8139",	"e1000",
 	NULL,
 };
 

src/init.c

@@ -28,23 +28,7 @@ void kmain()
 		preboot_fail("Failed to mount boot drive to VFS!");
 	}
 
-	/*module test;
-	fs_directory_entry_t * f = fs_get_file_info("/system/modules/test.ko");
-	if (f) {
-		dprintf("*** MOD TEST: Got info on test.ko size %lu\n", f->size);
-		void* module_content = kmalloc(f->size);
-		if (fs_read_file(f, 0, f->size, module_content)) {
-			dprintf("*** LOADING TEST MODULE test.ko ***\n");
-			if (module_load_from_memory(module_content, f->size, &test)) {
-				module_unload(&test);
-			}
-			dprintf("*** DONE WITH TEST MODULE test.ko ***\n");
-		} else {
-			dprintf("Couldnt read test.ko\n");
-		}
-	} else {
-		dprintf("*** MOD TEST: Cant get info on test.ko\n");
-	}*/
+	//load_module("test");
 
 	init_process();
 }