Skip to contentSkip to conversation

busybox: disable CONFIG_FEATURE_CLEAN_UP in default configs

PATCH New Peter Korsgaard 2 participants 3 messages Jul 5, 2017 – Jul 19, 2017

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 busybox: disable CONFIG_FEATURE_CLEAN_UP in default configs

FEATURE_CLEAN_UP is a configuration feature to get busybox to explicitly call free() on dynamic allocated memory just before exiting so memory leak detectors like valgrind don't get confused. Upstream explicitly recommends to NOT enable this option:

config FEATURE_CLEAN_UP
bool "Clean up all memory before exiting (usually not needed)" default n
help
As a size optimization, busybox normally exits without explicitly freeing dynamically allocated memory or closing files. This saves space since the OS will clean up for us, but it can confuse debuggers like valgrind, which report tons of memory and resource leaks.

Don't enable this unless you have a really good reason to clean things up manually.

Having this option enabled adds a bit of bloat, but more significantly these cleanup code paths don't get tested very often so some times get out of sync with the allocation code which can lead to crashes (or security issues from double frees), so it is safer to disable the option.

For people wanting to debug memory leak issues with busybox, the option can still be enabled with a configuration fragment (or a custom config).

The size difference isn't huge (br-arm-full-static):

-rwxr-xr-x 1 peko peko 886K Jul 5 10:56 output-busybox1/target/bin/busybox -rwxr-xr-x 1 peko peko 882K Jul 5 10:53 output-busybox2/target/bin/busybox

---
 package/busybox/busybox-minimal.config | 2 +-
 package/busybox/busybox.config         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
package/busybox/busybox-minimal.config +1 -1
index 2abf458b78..0c8e63e627 100644
@@ -22,7 +22,7 @@ CONFIG_FEATURE_INSTALLER=y
 # CONFIG_PAM is not set
 CONFIG_LONG_OPTS=y
 CONFIG_FEATURE_DEVPTS=y
-CONFIG_FEATURE_CLEAN_UP=y
+# CONFIG_FEATURE_CLEAN_UP is not set
 CONFIG_FEATURE_UTMP=y
 CONFIG_FEATURE_WTMP=y
 # CONFIG_FEATURE_PIDFILE is not set
package/busybox/busybox.config +1 -1
index c45de21808..508128d5e3 100644
@@ -22,7 +22,7 @@ CONFIG_FEATURE_INSTALLER=y
 # CONFIG_PAM is not set
 CONFIG_LONG_OPTS=y
 CONFIG_FEATURE_DEVPTS=y
-CONFIG_FEATURE_CLEAN_UP=y
+# CONFIG_FEATURE_CLEAN_UP is not set
 CONFIG_FEATURE_UTMP=y
 CONFIG_FEATURE_WTMP=y
 # CONFIG_FEATURE_PIDFILE is not set
Signature · 1 line
2.11.0
#1 lore raw

PATCH 1/1 busybox: disable CONFIG_FEATURE_CLEAN_UP in default configs

Hello,

Quoted 30 lines · Peter Korsgaard

FEATURE_CLEAN_UP is a configuration feature to get busybox to explicitly call free() on dynamic allocated memory just before exiting so memory leak detectors like valgrind don't get confused. Upstream explicitly recommends to NOT enable this option:

config FEATURE_CLEAN_UP
bool "Clean up all memory before exiting (usually not needed)" default n
help
As a size optimization, busybox normally exits without explicitly freeing dynamically allocated memory or closing files. This saves space since the OS will clean up for us, but it can confuse debuggers like valgrind, which report tons of memory and resource leaks.

Don't enable this unless you have a really good reason to clean things up manually.

Having this option enabled adds a bit of bloat, but more significantly these cleanup code paths don't get tested very often so some times get out of sync with the allocation code which can lead to crashes (or security issues from double frees), so it is safer to disable the option.

For people wanting to debug memory leak issues with busybox, the option can still be enabled with a configuration fragment (or a custom config).

The size difference isn't huge (br-arm-full-static):

(quoted diff) +0 -2
-rwxr-xr-x 1 peko peko 886K Jul  5 10:56 output-busybox1/target/bin/busybox
-rwxr-xr-x 1 peko peko 882K Jul  5 10:53 output-busybox2/target/bin/busybox
Quoted 5 lines
---
 package/busybox/busybox-minimal.config | 2 +-
 package/busybox/busybox.config         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Applied to master, thanks.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#2 lore raw

PATCH 1/1 busybox: disable CONFIG_FEATURE_CLEAN_UP in default configs

In reply to Peter Korsgaard
Quoted 32 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Peter" == Peter Korsgaard <[email protected]> writes:

FEATURE_CLEAN_UP is a configuration feature to get busybox to explicitly call free() on dynamic allocated memory just before exiting so memory leak detectors like valgrind don't get confused. Upstream explicitly recommends to NOT enable this option:

config FEATURE_CLEAN_UP
bool "Clean up all memory before exiting (usually not needed)" default n
help
As a size optimization, busybox normally exits without explicitly freeing dynamically allocated memory or closing files. This saves space since the OS will clean up for us, but it can confuse debuggers like valgrind, which report tons of memory and resource leaks.

Don't enable this unless you have a really good reason to clean things up manually.

Having this option enabled adds a bit of bloat, but more significantly these cleanup code paths don't get tested very often so some times get out of sync with the allocation code which can lead to crashes (or security issues from double frees), so it is safer to disable the option.

For people wanting to debug memory leak issues with busybox, the option can still be enabled with a configuration fragment (or a custom config).

The size difference isn't huge (br-arm-full-static):

(quoted diff) +0 -2
-rwxr-xr-x 1 peko peko 886K Jul  5 10:56 output-busybox1/target/bin/busybox
-rwxr-xr-x 1 peko peko 882K Jul  5 10:53 output-busybox2/target/bin/busybox
Quoted 1 line

Committed to 2017.02.x and 2017.05.x, thanks.

Signature · 1 line
Bye, Peter Korsgaard