fix invalid write in pop_cmd()

The memmove() in pop_cmd() reads and writes beyond the end of argv.

This is basically harmless in the current C program; the environment
variable list immediately follows argv so all this does is unnecessarily
copy the beginning of that list.

However, this will become problematic once we start calling C functions
like fs_cmds() from Rust code. Then argv will be a Vec<String> (as
*mut *mut i8) and the memory layout will be different--in particular,
I don't think we can assume that a Vec<String> will be NULL-terminated
like argv always is--, meaning the invalid write could lead to heap
corruption.

Also, it doesn't look like full_cmd ever gets used after calling
pop_cmd() so I'm removing it here since it looks unneeded to me.

Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

diff --git a/bcachefs.c b/bcachefs.c
index 7b8d4d648a4bc2ed2ee6b8bf7245304bf7cec24e..7ca79adab7108612d7b9825ccfa3099db5d72b01 100644 (file)
--- a/bcachefs.c
+++ b/bcachefs.c
@@ -104,16 +104,14 @@ static void usage(void)
             "  version                  Display the version of the invoked bcachefs tool\n");
 }
 
-static char *full_cmd;
-
 static char *pop_cmd(int *argc, char *argv[])
 {
        char *cmd = argv[1];
        if (!(*argc < 2))
-               memmove(&argv[1], &argv[2], *argc * sizeof(argv[0]));
+               memmove(&argv[1], &argv[2], (*argc - 2) * sizeof(argv[0]));
        (*argc)--;
+       argv[*argc] = NULL;
 
-       full_cmd = mprintf("%s %s", full_cmd, cmd);
        return cmd;
 }
 
@@ -190,7 +188,7 @@ int main(int argc, char *argv[])
 {
        raid_init();
 
-       full_cmd = argv[0];
+       char *full_cmd = argv[0];
 
        /* Are we being called via a symlink? */