On Sun, Aug 3, 2025 at 9:33 PM Harry Eaton <[email protected]> wrote: > > Attached is the patch file, and an "output" from the LEAK_HUNTING=1 on the > real target system that actually doesn't have an MMU where the first leak > (ptr_to_globals) is fixed (but with potential problems for sigexit(), but not > the other one. > > The first leak is straightforward, at the start of hush_main() it does > INIT_G() which does an xmalloc(), but there was never a corresponding free. > With an MMU, that's fine because everything disappears, but with NOMMU, it's > a problem.
No, it is not a problem. The kernel tears down all virtual memory areas of a dead process and frees all its memory, on MMU and on NOMMU. > So I added the free(ptr_to_globals) to solve that. It is inside the # if > ENABLE_FEATURE_CLEANUP, and because it is always necessary when there is no > MMU, busybox should probably automatically set ENABLE_FEATURE_CLEANUP if > BB_MMU is 0. > > Thanks for catching that sigexit() uses the globals. So it seems the thing to > do is to do instead is: > > @@ -2054,6 +2114,9 @@ > sigprocmask_allsigs(SIG_BLOCK); > tcsetpgrp(G_interactive_fd, G_saved_tty_pgrp); > } > +#if ENABLE_FEATURE_CLEAN_UP > + free (ptr_to_globals); > +#endif > > /* Not a signal, just exit */ > if (sig <= 0) > @@ -2135,6 +2198,9 @@ > > #if ENABLE_HUSH_JOB > sigexit(- (exitcode & 0xff)); > #else > + #if ENABLE_FEATURE_CLEAN_UP > + free (ptr_to_globals); > + #endif > _exit(exitcode); > #endif > } > > The other leak is difficult to explain "exactly". It mostly doesn't happen on > a system with a real MMU, but it happens reliably on my actual target which > has only 1 processor and no MMU. If you run the new hunt_leaktool.sh on the > attached "output.on_target.txt" it will show the leak. If you look carefully > at the output file, the free() is happening, but it is always 0, so it > doesn't free anything. I reproduced it. It's overzealous optimization. gcc just doesn't store the pointer into *to_free, because it can see that in all callers of the static function where the store is done, the address points to a local (on-stack) variable, therefore this variable is not visible to any other thread, and also the store can't alias with any global variables. And we are in a NORETURN function, so gcc can see the entire execution path until the program "ends" So, the store looks "dead" to gcc and it eliminates it. I discovered this when an added debugging statement took the address of the variable and passed it to a printf. The conclusion that the store is "dead" become invalid and the leak disappeared (gcc no longer eliminated the store). _______________________________________________ busybox mailing list [email protected] https://lists.busybox.net/mailman/listinfo/busybox
