child_vmsplice_memcmp_fn() allocated two heap buffers (old, new) and opened a pipe, but every return path - including the normal one - returned directly without freeing the buffers or closing the pipe file descriptors. It also used the malloc() results without checking them, so memcpy(old, mem, size) would crash on allocation failure (size can be as large as a huge page).
The sibling helper do_test_vmsplice_in_parent() in the same file already uses the goto-based cleanup pattern; follow it here. Check the allocations up front and introduce unified error handling with goto labels so that old/new are always freed and both pipe fds are always closed before returning, on every path. Signed-off-by: Jiangshan Yi <[email protected]> --- tools/testing/selftests/mm/cow.c | 44 ++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c index 0c627ea89ff7..f6a1bedfd6e2 100644 --- a/tools/testing/selftests/mm/cow.c +++ b/tools/testing/selftests/mm/cow.c @@ -145,26 +145,39 @@ static int child_vmsplice_memcmp_fn(char *mem, size_t size, char *old, *new; int fds[2]; char buf; + int ret; old = malloc(size); new = malloc(size); + if (!old || !new) { + ret = -ENOMEM; + goto free; + } /* Backup the original content. */ memcpy(old, mem, size); - if (pipe(fds) < 0) - return -errno; + if (pipe(fds) < 0) { + ret = -errno; + goto free; + } /* Trigger a read-only pin. */ transferred = vmsplice(fds[1], &iov, 1, 0); - if (transferred < 0) - return -errno; - if (transferred == 0) - return -EINVAL; + if (transferred < 0) { + ret = -errno; + goto close_pipe; + } + if (transferred == 0) { + ret = -EINVAL; + goto close_pipe; + } /* Unmap it from our page tables. */ - if (munmap(mem, size) < 0) - return -errno; + if (munmap(mem, size) < 0) { + ret = -errno; + goto close_pipe; + } /* Wait until the parent modified it. */ write(comm_pipes->child_ready[1], "0", 1); @@ -174,11 +187,20 @@ static int child_vmsplice_memcmp_fn(char *mem, size_t size, /* See if we still read the old values via the pipe. */ for (total = 0; total < transferred; total += cur) { cur = read(fds[0], new + total, transferred - total); - if (cur < 0) - return -errno; + if (cur < 0) { + ret = -errno; + goto close_pipe; + } } - return memcmp(old, new, transferred); + ret = memcmp(old, new, transferred); +close_pipe: + close(fds[0]); + close(fds[1]); +free: + free(old); + free(new); + return ret; } typedef int (*child_fn)(char *mem, size_t size, struct comm_pipes *comm_pipes); -- 2.25.1
