kmodloader: fix invalid write during insmod, CodeQL warnings

Running 'insmod <module>' (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 <itugrok@yahoo.com>

diff --git a/kmodloader.c b/kmodloader.c
index 12878391d2e54336e1ca595c5b49902106960a95..01fa8f6f3368a9eae779fca4d3aea9cd10c3ecb3 100644 (file)
--- 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();