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 <[email protected]> --- bcachefs.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/bcachefs.c b/bcachefs.c index 7b8d4d6..7ca79ad 100644 --- 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? */ -- 2.43.0
