I have solved the leak problem. This patch (to busybox-1.37.0) solves the
leaks when compiled with NOMMU
(change the #define BUILD_AS_NOMMU line from 0 to 1 in order to demonstate
that the unpatch code leaks,
while the patched code does not on a system that actually has an MMU since
most developers don't have
an available system that actually lacks an MMU. This code in unpatched hush
with NO_MMU will cause the
leak that can be seen with top:
while true; do v=$(echo 0); done
Below is the patch that only fixes the leak.
------------------------------------------------ diff
-----------------------------------------------------
-- hush.c.orig 2025-07-28 17:57:34.808579711 -0400
+++ hush.c 2025-08-01 15:24:14.885718809 -0400
@@ -961,6 +961,9 @@
#if ENABLE_HUSH_GETOPTS
unsigned getopt_count;
#endif
+#if ENABLE_HUSH_TICK
+ char **to_free;
+#endif
const char *ifs;
char *ifs_whitespace; /* = G.ifs or malloced */
const char *cwd;
@@ -2128,6 +2131,9 @@
cur_var = cur_var->next;
free(tmp);
}
+ free(G.to_free);
+ G.to_free = NULL;
+ free(ptr_to_globals);
}
#endif
@@ -7657,9 +7663,6 @@
{
pid_t pid;
int channel[2];
-# if !BB_MMU
- char **to_free = NULL;
-# endif
xpipe(channel);
pid = BB_MMU ? xfork() : xvfork();
@@ -7736,7 +7739,7 @@
* huge=`cat BIG` # was blocking here forever
* echo OK
*/
- re_execute_shell(&to_free,
+ re_execute_shell(&G.to_free,
s,
G.global_argv[0],
G.global_argv + 1,
@@ -7754,7 +7757,8 @@
# endif
enable_restore_tty_pgrp_on_exit();
# if !BB_MMU
- free(to_free);
+ free(G.to_free);
+ G.to_free = NULL;
--------------------------------------------------------------------------------------------------
In order to discover these bugs, I had to greatly improve the LEAK_HUNTING
code in hush.c
as well as make a different, better hush_leaktool.sh.
Below is the patch that includes the improvements to LEAK_HUNTING as well
as the bug fix.
------------------------------------------ diff
--------------------------------------------------------
--- hush.c.orig 2025-08-01 13:31:24.541695883 -0400
+++ hush.c 2025-08-01 13:37:44.957697171 -0400
@@ -961,6 +961,9 @@
#if ENABLE_HUSH_GETOPTS
unsigned getopt_count;
#endif
+#if ENABLE_HUSH_TICK
+ char **to_free;
+#endif
const char *ifs;
char *ifs_whitespace; /* = G.ifs or malloced */
const char *cwd;
@@ -1343,36 +1346,157 @@
#endif
+static char **add_strings_to_strings(char **strings, char **add, int
need_to_dup)
+{
+ int i;
+ unsigned count1;
+ unsigned count2;
+ char **v;
+
+ v = strings;
+ count1 = 0;
+ if (v) {
+ while (*v) {
+ count1++;
+ v++;
+ }
+ }
+ count2 = 0;
+ v = add;
+ while (*v) {
+ count2++;
+ v++;
+ }
+ v = xrealloc(strings, (count1 + count2 + 1) * sizeof(char*));
+ v[count1 + count2] = NULL;
+ i = count2;
+ while (--i >= 0)
+ v[count1 + i] = (need_to_dup ? xstrdup(add[i]) : add[i]);
+ return v;
+}
+
+/* Note: takes ownership of "add" ptr (it is not strdup'ed) */
+static char **add_string_to_strings(char **strings, char *add)
+{
+ char *v[2];
+ v[0] = add;
+ v[1] = NULL;
+ return add_strings_to_strings(strings, v, /*dup:*/ 0);
+}
+
/* Leak hunting. Use hush_leaktool.sh for post-processing.
*/
+
#if LEAK_HUNTING
-static void *xxmalloc(int lineno, size_t size)
+static char **xxadd_strings_to_strings(int lineno, char **strings, char
**add, int need_to_dup)
+{
+ char **orig = strings;
+ char **ptr = add_strings_to_strings(strings, add, need_to_dup);
+ if (orig != ptr) {
+ fdprintf(2, "%p line %d: add_strings_to_strings\n", ptr, lineno);
+ if (orig != NULL) fdprintf(2, "free %p by add_strings_to_strings\n",
orig);
+ fflush_all();
+ }
+ return ptr;
+}
+
+static char **xxadd_string_to_strings(int lineno, char **strings, char
*add)
+{
+ char **orig = strings;
+ char **ptr = add_string_to_strings(strings, add);
+ if (ptr != orig) {
+ fdprintf(2, "%p line %d: add_string_to_strings\n", ptr, lineno);
+ if (orig != NULL) fdprintf(2, "free %p by add_string_to_string\n", orig);;
+ fflush_all();
+ }
+ return ptr;
+}
+
+static void *xxmalloc(int lineno, const char *s, size_t size)
{
void *ptr = xmalloc((size + 0xff) & ~0xff);
- fdprintf(2, "line %d: malloc %p\n", lineno, ptr);
+ fdprintf(2, "%p line %d: malloc(%s)\n", ptr, lineno, s);
+ fflush_all();
+ return ptr;
+}
+static void *xxzalloc(int lineno, const char *s, size_t size)
+{
+ void *ptr = xzalloc((size + 0xff) & ~0xff);
+ fdprintf(2, "%p line %d: zalloc(%s)\n", ptr, lineno, s);
+ fflush_all();
return ptr;
}
-static void *xxrealloc(int lineno, void *ptr, size_t size)
+static void *xxrealloc(int lineno, const char *s, void *ptr, size_t size)
{
+ void *p = ptr;
ptr = xrealloc(ptr, (size + 0xff) & ~0xff);
- fdprintf(2, "line %d: realloc %p\n", lineno, ptr);
+ if (p != ptr) {
+ fdprintf(2, "%p line %d: realloc(%s)\n", ptr, lineno, s);
+ if (p != NULL) fdprintf(2, "free %p by realloc\n", p);
+ fflush_all();
+ }
+ return ptr;
+}
+static void *xxrealloc_getcwd_or_warn(int lineno, char *ptr)
+{
+ char *p = ptr;
+ ptr = xrealloc_getcwd_or_warn(ptr);
+ if (p != ptr) {
+ fdprintf(2, "%p line %d: xrealloc_getcwd_or_warn\n", ptr, lineno);
+ if (p != NULL) fdprintf(2, "free %p by getcwd_or_warn\n", p);
+ fflush_all();
+ }
return ptr;
}
-static char *xxstrdup(int lineno, const char *str)
+static char *xxstrdup(int lineno, const char *s, const char *str)
{
char *ptr = xstrdup(str);
- fdprintf(2, "line %d: strdup %p\n", lineno, ptr);
+ fdprintf(2, "%p line %d: strdup(%s) %s\n", ptr, lineno, s, ptr);
+ fflush_all();
+ return ptr;
+}
+static char *xxstrndup(int lineno, const char *s, const char *str, int n)
+{
+ char *ptr = xstrndup(str, n);
+ fdprintf(2, "%p line %d: strndup(%s) %s\n", ptr, lineno, s, ptr);
+ fflush_all();
return ptr;
}
-static void xxfree(void *ptr)
+static char *xxasprintf(int lineno, const char *f, ...)
{
- fdprintf(2, "free %p\n", ptr);
+ int r;
+ char *p;
+ va_list args;
+ va_start(args, f);
+ r = vasprintf(&p, f, args);
+ va_end(args);
+ if (r < 0)
+ bb_die_memory_exhausted();
+ fdprintf(2, "%p line %d: xasprintf %s\n", p, lineno, p);
+ fflush_all();
+ return p;
+}
+static void xxfree(int lineno, const char *s, void *ptr)
+{
+ if (ptr) {
+ fdprintf(2, "free %p line %d (%s)\n", ptr, lineno, s);
+ fflush_all();
free(ptr);
+ } else fdprintf(2, "free null line %d (%s)\n", lineno, s);
}
-# define xmalloc(s) xxmalloc(__LINE__, s)
-# define xrealloc(p, s) xxrealloc(__LINE__, p, s)
-# define xstrdup(s) xxstrdup(__LINE__, s)
-# define free(p) xxfree(p)
+#define STRINGIFY(x) #x
+#define add_strings_to_strings(strings, add, need_to_dup) \
+ xxadd_strings_to_strings(__LINE__, strings, add, need_to_dup)
+#define add_string_to_strings(strings, add) \
+ xxadd_string_to_strings(__LINE__, strings, add)
+# define xmalloc(s) xxmalloc(__LINE__, STRINGIFY(s), s)
+# define xzalloc(s) xxzalloc(__LINE__, STRINGIFY(s), s)
+# define xrealloc(p, s) xxrealloc(__LINE__, STRINGIFY(p), p, s )
+# define xstrdup(s) xxstrdup(__LINE__, STRINGIFY(s), s)
+# define xstrndup(s, n) xxstrndup(__LINE__, STRINGIFY(s), s, n)
+# define xrealloc_getcwd_or_warn(p) xxrealloc_getcwd_or_warn(__LINE__, p)
+# define xasprintf(f, ...) xxasprintf(__LINE__, f, __VA_ARGS__)
+# define free(p) xxfree(__LINE__, STRINGIFY(p), p)
#endif
@@ -1496,64 +1620,6 @@
return dst;
}
-static char **add_strings_to_strings(char **strings, char **add, int
need_to_dup)
-{
- int i;
- unsigned count1;
- unsigned count2;
- char **v;
-
- v = strings;
- count1 = 0;
- if (v) {
- while (*v) {
- count1++;
- v++;
- }
- }
- count2 = 0;
- v = add;
- while (*v) {
- count2++;
- v++;
- }
- v = xrealloc(strings, (count1 + count2 + 1) * sizeof(char*));
- v[count1 + count2] = NULL;
- i = count2;
- while (--i >= 0)
- v[count1 + i] = (need_to_dup ? xstrdup(add[i]) : add[i]);
- return v;
-}
-#if LEAK_HUNTING
-static char **xx_add_strings_to_strings(int lineno, char **strings, char
**add, int need_to_dup)
-{
- char **ptr = add_strings_to_strings(strings, add, need_to_dup);
- fdprintf(2, "line %d: add_strings_to_strings %p\n", lineno, ptr);
- return ptr;
-}
-#define add_strings_to_strings(strings, add, need_to_dup) \
- xx_add_strings_to_strings(__LINE__, strings, add, need_to_dup)
-#endif
-
-/* Note: takes ownership of "add" ptr (it is not strdup'ed) */
-static char **add_string_to_strings(char **strings, char *add)
-{
- char *v[2];
- v[0] = add;
- v[1] = NULL;
- return add_strings_to_strings(strings, v, /*dup:*/ 0);
-}
-#if LEAK_HUNTING
-static char **xx_add_string_to_strings(int lineno, char **strings, char
*add)
-{
- char **ptr = add_string_to_strings(strings, add);
- fdprintf(2, "line %d: add_string_to_strings %p\n", lineno, ptr);
- return ptr;
-}
-#define add_string_to_strings(strings, add) \
- xx_add_string_to_strings(__LINE__, strings, add)
-#endif
-
static void free_strings(char **strings)
{
char **v;
@@ -2128,6 +2194,10 @@
cur_var = cur_var->next;
free(tmp);
}
+ free(G.to_free);
+ G.to_free = 0;
+ free(ptr_to_globals);
}
#endif
@@ -7658,7 +7728,7 @@
pid_t pid;
int channel[2];
# if !BB_MMU
- char **to_free = NULL;
+ G.to_free = NULL;
# endif
xpipe(channel);
@@ -7736,7 +7806,7 @@
* huge=`cat BIG` # was blocking here forever
* echo OK
*/
- re_execute_shell(&to_free,
+ re_execute_shell(&G.to_free,
s,
G.global_argv[0],
G.global_argv + 1,
@@ -7754,7 +7824,8 @@
# endif
enable_restore_tty_pgrp_on_exit();
# if !BB_MMU
- free(to_free);
+ free(G.to_free);
+ G.to_free = NULL;
# endif
close(channel[1]);
return channel[0];
____________________________________________________________________
Below is the new hush_leaktool.sh
----------------------------------------------- diff
--------------------------------------------------------------
#!/bin/bash
# hush_leaktool.sh
# works with GNU sed, maybe not busybox's sed.
# hush's stderr with leak debug enabled
output=output
grep -v '^free' "$output" > "$output.leaked"
grep '^free 0x' "$output" > "$output.spurious_free"
grep '^0x' "$output" > "$output.alloc"
alloclist=`cut -d' ' -f1 "$output.alloc" | xargs`
for mem in $alloclist; do
if grep -q $mem "$output.spurious_free"; then
# echo "$mem freed"
# drop first corresponding free
sed -i '0,/'$mem'/{/'$mem'/d}' "$output.spurious_free"
# drop from the leaked list
sed -i '0,/'$mem'/{/'$mem'/d}' "$output.leaked"
fi
done
cat "$output.leaked"
_______________________________________________________________________
Finally, mea culpa in my first message: the leak had nothing to do with
looping. I didn't
realize that hush didn't treat {1..10} like bash does. Also my leak hunting
improvements
still had bugs that led to false positives (but that's fixed now), so my
clues were wrong.
I wasn't wrong about the original leak_hunting code being able to hide all
kinds of leaks.
harry
_______________________________________________
busybox mailing list
[email protected]
https://lists.busybox.net/mailman/listinfo/busybox
