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@kylinos.cn Suggested-by: Ming Xie xieming@kylinos.cn --- 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);
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@kylinos.cn Suggested-by: Ming Xie xieming@kylinos.cn
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.
linux-kselftest-mirror@lists.linaro.org