On 04.04.23 05:12, Feng Jiang wrote:
When the function returns, the 'new' and 'old' are not freed
and the 'fds[]' are not closed, which can lead to resource leaks.
Signed-off-by: Feng Jiang <jiangfeng@xxxxxxxxxx>
Suggested-by: Ming Xie <xieming@xxxxxxxxxx>
---
tools/testing/selftests/mm/cow.c | 43 ++++++++++++++++++++++++--------
1 file changed, 32 insertions(+), 11 deletions(-)
diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c
index c0dd2dfca51b..b8aec05d56f4 100644
--- a/tools/testing/selftests/mm/cow.c
+++ b/tools/testing/selftests/mm/cow.c
@@ -193,26 +193,38 @@ 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;
+ } else 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);
@@ -222,11 +234,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);
NAK, the whole point of this function is that the child process will
exit immediately after executing this function, cleaning up automatically.
--
Thanks,
David / dhildenb
