diff mbox series

[v2] libkmod: Do not inititialize file->memory on open

Message ID 20230616050041.99712-1-lucas.de.marchi@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v2] libkmod: Do not inititialize file->memory on open | expand

Commit Message

Lucas De Marchi June 16, 2023, 5 a.m. UTC 
Add a separate function to load the file contents when it's needed.
When it's not needed on the path of loading modules via finit_module(),
there is no need to mmap the file. This will help support loading
modules with the in-kernel compression support.

This is done differently than the lazy initialization for
kmod_file_get_elf() because on the contents case there is also the
file->size to be updated. It would be a weird API to return the pointer
and have the size changed as a side-effect.

Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
---

v2: change kmod_file_load_contents() to void since the return is
    actually checked on the file struct

 libkmod/libkmod-elf.c      |  5 +++++
 libkmod/libkmod-file.c     | 21 ++++++++++++++++++---
 libkmod/libkmod-internal.h |  3 ++-
 libkmod/libkmod-module.c   |  2 ++
 4 files changed, 27 insertions(+), 4 deletions(-)

Comments

Lucas De Marchi July 24, 2023, 1:29 p.m. UTC | #1 
On Thu, Jun 15, 2023 at 10:00:41PM -0700, Lucas De Marchi wrote:
>Add a separate function to load the file contents when it's needed.
>When it's not needed on the path of loading modules via finit_module(),
>there is no need to mmap the file. This will help support loading
>modules with the in-kernel compression support.
>
>This is done differently than the lazy initialization for
>kmod_file_get_elf() because on the contents case there is also the
>file->size to be updated. It would be a weird API to return the pointer
>and have the size changed as a side-effect.
>
>Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
>---
>
>v2: change kmod_file_load_contents() to void since the return is
>    actually checked on the file struct

pushed with this change.

Lucas De Marchi
diff mbox series

Patch

diff --git a/libkmod/libkmod-elf.c b/libkmod/libkmod-elf.c
index fb2e3d9..933825b 100644
--- a/libkmod/libkmod-elf.c
+++ b/libkmod/libkmod-elf.c
@@ -281,6 +281,11 @@  struct kmod_elf *kmod_elf_new(const void *memory, off_t size)
 	assert_cc(sizeof(uint32_t) == sizeof(Elf32_Word));
 	assert_cc(sizeof(uint32_t) == sizeof(Elf64_Word));
 
+	if (!memory) {
+		errno = -EINVAL;
+		return NULL;
+	}
+
 	class = elf_identify(memory, size);
 	if (class < 0) {
 		errno = -class;
diff --git a/libkmod/libkmod-file.c b/libkmod/libkmod-file.c
index b6a8cc9..08adea9 100644
--- a/libkmod/libkmod-file.c
+++ b/libkmod/libkmod-file.c
@@ -421,6 +421,7 @@  struct kmod_elf *kmod_file_get_elf(struct kmod_file *file)
 	if (file->elf)
 		return file->elf;
 
+	kmod_file_load_contents(file);
 	file->elf = kmod_elf_new(file->memory, file->size);
 	return file->elf;
 }
@@ -431,7 +432,7 @@  struct kmod_file *kmod_file_open(const struct kmod_ctx *ctx,
 	struct kmod_file *file = calloc(1, sizeof(struct kmod_file));
 	const struct comp_type *itr;
 	size_t magic_size_max = 0;
-	int err;
+	int err = 0;
 
 	if (file == NULL)
 		return NULL;
@@ -477,8 +478,8 @@  struct kmod_file *kmod_file_open(const struct kmod_ctx *ctx,
 	if (file->ops == NULL)
 		file->ops = &reg_ops;
 
-	err = file->ops->load(file);
 	file->ctx = ctx;
+
 error:
 	if (err < 0) {
 		if (file->fd >= 0)
@@ -491,6 +492,18 @@  error:
 	return file;
 }
 
+/*
+ *  Callers should just check file->memory got updated
+ */
+void kmod_file_load_contents(struct kmod_file *file)
+{
+	if (file->memory)
+		return;
+
+	/*  The load functions already log possible errors. */
+	file->ops->load(file);
+}
+
 void *kmod_file_get_contents(const struct kmod_file *file)
 {
 	return file->memory;
@@ -516,7 +529,9 @@  void kmod_file_unref(struct kmod_file *file)
 	if (file->elf)
 		kmod_elf_unref(file->elf);
 
-	file->ops->unload(file);
+	if (file->memory)
+		file->ops->unload(file);
+
 	if (file->fd >= 0)
 		close(file->fd);
 	free(file);
diff --git a/libkmod/libkmod-internal.h b/libkmod/libkmod-internal.h
index 4a4af58..3275bc5 100644
--- a/libkmod/libkmod-internal.h
+++ b/libkmod/libkmod-internal.h
@@ -152,6 +152,7 @@  bool kmod_module_is_builtin(struct kmod_module *mod) __attribute__((nonnull(1)))
 /* libkmod-file.c */
 struct kmod_file *kmod_file_open(const struct kmod_ctx *ctx, const char *filename) _must_check_ __attribute__((nonnull(1,2)));
 struct kmod_elf *kmod_file_get_elf(struct kmod_file *file) __attribute__((nonnull(1)));
+void kmod_file_load_contents(struct kmod_file *file) __attribute__((nonnull(1)));
 void *kmod_file_get_contents(const struct kmod_file *file) _must_check_ __attribute__((nonnull(1)));
 off_t kmod_file_get_size(const struct kmod_file *file) _must_check_ __attribute__((nonnull(1)));
 bool kmod_file_get_direct(const struct kmod_file *file) _must_check_ __attribute__((nonnull(1)));
@@ -166,7 +167,7 @@  struct kmod_modversion {
 	char *symbol;
 };
 
-struct kmod_elf *kmod_elf_new(const void *memory, off_t size) _must_check_ __attribute__((nonnull(1)));
+struct kmod_elf *kmod_elf_new(const void *memory, off_t size) _must_check_;
 void kmod_elf_unref(struct kmod_elf *elf) __attribute__((nonnull(1)));
 const void *kmod_elf_get_memory(const struct kmod_elf *elf) _must_check_ __attribute__((nonnull(1)));
 int kmod_elf_get_strings(const struct kmod_elf *elf, const char *section, char ***array) _must_check_ __attribute__((nonnull(1,2,3)));
diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c
index 7736b7e..f352fe1 100644
--- a/libkmod/libkmod-module.c
+++ b/libkmod/libkmod-module.c
@@ -917,6 +917,8 @@  KMOD_EXPORT int kmod_module_insert_module(struct kmod_module *mod,
 			goto init_finished;
 	}
 
+	kmod_file_load_contents(mod->file);
+
 	if (flags & (KMOD_INSERT_FORCE_VERMAGIC | KMOD_INSERT_FORCE_MODVERSION)) {
 		elf = kmod_file_get_elf(mod->file);
 		if (elf == NULL) {