| Message ID | 20231121025506.2391-1-zhujun2@cmss.chinamobile.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/memfd: fix a memleak | expand |
On Mon, 20 Nov 2023 18:55:06 -0800 zhujun2 <zhujun2@cmss.chinamobile.com> wrote: > The memory allocated within a function should be released > before the function return,otherwise memleak will occur. > > ... > > --- a/tools/testing/selftests/memfd/fuse_test.c > +++ b/tools/testing/selftests/memfd/fuse_test.c > @@ -217,6 +217,7 @@ static pid_t spawn_sealing_thread(void) > abort(); > } > > + free(stack); > return pid; > } We just freed a thread's stack while it is still running?
On 2023-11-21 18:43, Andrew Morton wrote: > On Mon, 20 Nov 2023 18:55:06 -0800 zhujun2 <zhujun2@cmss.chinamobile.com> wrote: > >> The memory allocated within a function should be released >> before the function return,otherwise memleak will occur. >> >> ... >> >> --- a/tools/testing/selftests/memfd/fuse_test.c >> +++ b/tools/testing/selftests/memfd/fuse_test.c >> @@ -217,6 +217,7 @@ static pid_t spawn_sealing_thread(void) >> abort(); >> } >> >> + free(stack); >> return pid; >> } > > We just freed a thread's stack while it is still running? Indeed, this patch is completely wrong. Freeing the stack memory should not happen before joining the thread which uses it. This also means that this patch was probably not even runtime-tested, even less tested with tools like valgrind which should have caught this. I would advise the patch author to test the changes before submitting a patch, especially considering the amount of tools out there which can be used to validate userspace programs. Moreover, the changes done should take into consideration the behavior of the system calls they surround. Trying to "fix" a resource leak without understanding first the semantic of the system call using the resource is causing more problems than it solves. Thanks, Mathieu
diff --git a/tools/testing/selftests/memfd/fuse_test.c b/tools/testing/selftests/memfd/fuse_test.c index 93798c8c5d54..fd934487c528 100644 --- a/tools/testing/selftests/memfd/fuse_test.c +++ b/tools/testing/selftests/memfd/fuse_test.c @@ -217,6 +217,7 @@ static pid_t spawn_sealing_thread(void) abort(); } + free(stack); return pid; }
The memory allocated within a function should be released before the function return,otherwise memleak will occur. Signed-off-by: zhujun2 <zhujun2@cmss.chinamobile.com> --- tools/testing/selftests/memfd/fuse_test.c | 1 + 1 file changed, 1 insertion(+)