Passing a long argument to execve crashes the kernel

[rick-masters]

The following test program should crash Fiwix:

#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>

char longarg[128 * 4096 + 1];

int main() {
        char *argv[] = { "cat", NULL, NULL };
        char *envp[] = { NULL };
        memset(longarg, 'a', 128 * 4096);
        argv[1] = longarg;
        int err = execve("cat", argv, envp);
        printf("errno is %d\n", errno);
        printf("err is %d\n", err);
}

While there is argument and environment length checking in Fiwix, the checking is done too late to prevent the problem.

The GNU autoconf tool can perform checks that try to determine the maximum argument length that can be passed to a program, which triggers this crash.

A PR with a suggested fix is forthcoming.

[mikaku]

I thought that with this check in elf_load() was enough to prevent the problem:

https://github.com/mikaku/Fiwix/blob/1c24d97982efe9e29aab7fcb0c033bda47dbe7a3/fs/elf.c#L460-L464

During a massive package build, one of the checks from some ./configure scripts is, as you said, to determine the maximum argument length supported by the kernel, and so you can see a number of messages like these:

WARNING: elf_load(): argument list (133042) exceeds ARG_MAX (131072)!
WARNING: elf_load(): argument list (133038) exceeds ARG_MAX (131072)!
WARNING: elf_load(): argument list (133016) exceeds ARG_MAX (131072)!
...

(this WARNING message will be removed in the future).

Besides this warning, there was no crash at all. That's what made me think that this problem was under control.

I have executed your test program, which initially it returned no such file or directory until I changed cat by /bin/cat, and I see this:

[lots of 'a' ...]
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: File or path name too long

There is no crash. Perhaps this test program is not the latest version you tested?

[rick-masters]

Thanks for looking at this. I just retested with the latest Fiwix code. Now, I'm seeing a page fault and backtrace. The crash/reboot is not happening right now and may have been specific to my situation when I originally ran into this, perhaps the compiler I was using or modified/older version of Fiwix.

It can be seen by inspection of do_execve that copy_strings happens before elf_load and copy_strings has no length checking.

Perhaps we can work together to figure out how to trigger a fault in your test scenario. However, I'm just leaving to go skiing today so we may have to do that tomorrow. Perhaps you could try longer arguments?

[mikaku]

I'll try with different argument sizes to see if I can get a crash. Anyway, the bug may exist as copy_strings() is not taking care of the length of the arguments.

Have fun!

[mikaku]

Indeed, using greater values is easier to see the crash (actually the program segfault-ed and is terminated by the kernel). Thank you very much for fixing this bug.