From 4c5c45c6beac586c994af34a795bc5f2232f5f3d Mon Sep 17 00:00:00 2001 From: Tony Ambardar Date: Wed, 24 Jan 2024 18:22:56 -0800 Subject: [PATCH] kmodloader: fix invalid write during insmod, CodeQL warnings Running 'insmod ' (without module options) tries to write an empty option string (1 byte) to a mis-allocated zero-size memory block: ==381835== Command: ./insmod unix ==381835== ==381835== Invalid write of size 1 ==381835== at 0x10A874: main_insmod (kmodloader.c:944) ==381835== by 0x10A874: main (kmodloader.c:1383) ==381835== Address 0x4ab6f60 is 0 bytes after a block of size 0 alloc'd ==381835== at 0x4848899: malloc (in vgpreload_memcheck-amd64-linux.so) ==381835== by 0x10A856: main_insmod (kmodloader.c:937) ==381835== by 0x10A856: main (kmodloader.c:1383) Change main_insmod() to allocate at least 1 byte in this case for the null-termination. Also rename local 'options' -> 'opts' to avoid confusion with similar global, as suggested by CodeQL, and clarify pathname logic. Fixes: c105f22064d6 ("kmodloader: eliminate some hardcoded buffer sizes") Signed-off-by: Tony Ambardar --- kmodloader.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/kmodloader.c b/kmodloader.c index 1287839..01fa8f6 100644 --- a/kmodloader.c +++ b/kmodloader.c @@ -892,7 +892,7 @@ static int print_usage(char *arg) static int main_insmod(int argc, char **argv) { - char *name, *cur, *options = NULL; + char *name, *path, *cur, *opts = NULL; int i, ret = -1, len; if (argc < 2) @@ -913,19 +913,19 @@ static int main_insmod(int argc, char **argv) } - for (len = 0, i = 2; i < argc; i++) + for (len = 1, i = 2; i < argc; i++) len += strlen(argv[i]) + 1; - options = malloc(len); - if (!options) { + opts = malloc(len); + if (!opts) { ULOG_ERR("out of memory\n"); goto err; } - options[0] = 0; - cur = options; + opts[0] = 0; + cur = opts; for (i = 2; i < argc; i++) { - if (options[0]) { + if (opts[0]) { *cur = ' '; cur++; } @@ -937,19 +937,18 @@ static int main_insmod(int argc, char **argv) goto err; } - if (get_module_path(argv[1])) { - name = argv[1]; - } else if (!get_module_path(name)) { + if (!(path = get_module_path(argv[1])) || + (path = get_module_path(name))) { fprintf(stderr, "Failed to find %s. Maybe it is a built in module ?\n", name); goto err; } - ret = insert_module(get_module_path(name), options); + ret = insert_module(path, opts); if (ret) ULOG_ERR("failed to insert %s\n", get_module_path(name)); err: - free(options); + free(opts); free_modules(); free_module_folders(); -- 2.30.2