Inappropriate Use of realloc Leading to Memory Leak and Potential Null Pointer Dereference

[zkh8227]

#Issue Description There is an inappropriate use of realloc in contrib/fsstress.c, similar to the pattern p = realloc(p, size). When realloc fails, it returns NULL while leaving the original memory block intact. However, at this point, p is assigned NULL, making it impossible to free the original memory block. Moreover, if there is no null pointer check for p, subsequent operations may lead to null pointer dereference (causing a crash) and other issues.

#Detail https://github.com/tytso/e2fsprogs/blob/master/contrib/fsstress.c#L373 int main(int argc, char **argv) { ... ilist = realloc(ilist, ++ilistlen * sizeof(*ilist)); ilist[ilistlen - 1] = strtol(optarg, &p, 16); .... } When realloc fails and returns NULL in the above code, the ilist pointer will be set to NULL. This means that the original memory block pointed to by ilist cannot be freed, resulting in a memory leak. Additionally, the subsequent operation ilist[ilistlen - 1] = strtol(optarg, &p, 16); will access a NULL pointer, which will lead to undefined behavior and potentially crash the program.

[tytso]

fsstress.c is a test program which used for file system testing (in the kernel). It was added to e2fsck's contrib directory as a convenience for people who are building e2fsprogs for Android (and it is only used for Android). Since it is a test program, concerns of security issues if there are undefined issues aren't really an issue, since it is generally only run on test systems and never in production workloads, and it's generally run as root when it is run. Since this is in the code which parses options passed on the command-line, it is run at the very beginning of fsstress, and if realloc() is failing, that means memory is so tight that the correct functioning of fsstress is ultimately doomed. So this is not a particularly high priority issue to fix. It's more of something to shut up optional source code scanners.

This issue is still there in the ltp project and in the xfstests-dev project, which is the primary place where fsstress.c gets used. (I'm not sure the Android folks are using the version of fsstress in e2fsprogs at all, actually.) So it should probably be fixed there, and then we should either (a) update fsstress.c in e2fsprogs from one of the upstream sources of fsstress.c, or (b) just delete it from e2fsprogs if it's not actually used by Android developers.