Leak fixes#596
Draft
quitschbo wants to merge 9 commits into
Draft
Conversation
cmld_wrapped_keys_path is allocated via mem_printf() in cmld_init_stage_container() but was never released, leaking the asprintf-allocated string on daemon exit. Change the declaration from const char * to char * to match the mem_printf() return type (cmld_get_wrapped_keys_dir() keeps its const char * return type via implicit conversion), and release the buffer in cmld_cleanup() alongside the other path globals. Fixes: 4977717 ("daemon: Moved smartcard module to c_smartcard module") Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
unit.c keeps a static list of registered unit compartment modules. Modules are registered at INIT time via unit_register_compartment_module() and the list_t nodes were never freed at shutdown. Add unit_cleanup() which list_delete()s the list (the module data pointers themselves reference file-scope statics in the u_* modules and must not be freed), and register it via atexit() from cmld_init_stage_unit() so the nodes are released on cmld exit. Fixes: fb6de43 ("daemon/unit: Introduce new unit module for minimal compartments") Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
CONTAINER_MODULE_REGISTER_WRAPPER_IMPL allocates a small handler struct via mem_new0() for every container_register_*_handler() call. About 40 such handlers are registered during cmld startup (one per implementation in c_user, c_smartcard, c_cgroups_v2, c_net, c_vol, c_audit, c_run, c_time, c_cap, c_idmapped, c_service and c_cgroups_dev), and none of them were ever released. Track each newly-allocated handler in a static list inside container.c via container_module_track_handler(), invoked from the macro right after the mem_new0(). Add container_cleanup() which walks the tracking list and mem_free0()s each handler, then list_delete()s both the handler tracking list and the compartment_module_list. The module data pointers themselves are file-scope statics in the c_* modules and must not be freed. Register container_cleanup() via atexit() from cmld_init_stage_unit(). Ordering is safe: cmld_cleanup() (which calls container_free() -> compartment_free() -> module hooks) runs before container_cleanup() in the LIFO atexit chain, so handler dereferences during compartment teardown still see live handler structs. Fixes: f728f8b ("daemon/container: Added macro magic for module wrapper implementation") Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
compartment_new() builds compartment->description via mem_printf()
("name (uuid)") but compartment_free() never released it, leaking one
asprintf-allocated string per compartment lifetime.
The leak predates the container/compartment split; the comment above
the allocation already warned that future uuid/name setters must update
description, but no setter ever did, so the field has always been a
write-once mem_printf result that nothing reclaims.
Add the mem_free0() alongside the other char* fields freed in
compartment_free().
Fixes: ed54281 ("trustme github release (based on Android 5.1.1)")
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
compartment_unregister_observer() releases an individual compartment_callback_t via mem_free0() when it is removed from the list, but compartment_free() never iterates the residual observers. Any callback still registered when a compartment is destroyed leaks its struct (and the list_t node holding it). In practice this triggers on every cmld shutdown: the config-sync observer registered via container_register_observer() in cmld_reload_container_internal() is never unregistered for the container's lifetime, and additional observers may be left over when a container is torn down early (e.g. a failed network move during start, where some but not all observers have been removed). Walk observer_list before freeing the compartment and release each callback, then list_delete() the list. Fixes: ed54281 ("trustme github release (based on Android 5.1.1)") Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
container_config_get_dev_allow_list_new() and container_config_get_dev_assign_list_new() return NULL-terminated char ** arrays where both the outer array and every entry are heap-allocated (mem_new0 + mem_strdup per element). container_free() only released the outer array via mem_free0(), so every per-entry mem_strdup() (one per rule in <allow_dev> / <assign_dev>) leaked when the container was destroyed. Walk each list and mem_free0() the entry strings before releasing the array. The trailing NULL terminator placed by the _new() helpers makes the for-loop bound implicit. Fixes: beb7f2c ("whitelist based configuration rules to allow containers (exclusive or non-exclusive) access to hardware devices") Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
main() allocates two event_signal_t handlers (SIGINT and SIGTERM) and an event_timer_t for the periodic logfile rename. Their lifetime is "until the process exits", but cmld_handle_device_shutdown() short-circuits the event loop by calling exit(0) directly, so event_remove_signal()/event_remove_timer() are never invoked and the structs plus their list_t nodes in event_signal_list/event_timer_list are leaked at shutdown. Promote sig_int, sig_term and logfile_timer to file-scope statics and add main_event_cleanup() which removes and frees each, registered via atexit() right after the handlers are added. main_event_cleanup() runs before main_logf_cleanup() in LIFO order, keeping the log path available for any teardown logging. Fixes: ed54281 ("trustme github release (based on Android 5.1.1)") Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
container_vnet_cfg_new() mem_strdup()s rootns_name when present (e.g. the host0 vnet of testcontainer, or vnet0/vnet1 of containers that specify an rootns binding), but the cleanup loop in container_free() only released vnet_name and the cfg struct itself, leaking one mem_strdup per vnet on every container teardown. Add the missing mem_free0(vnet_cfg->rootns_name); mem_free0() is NULL-safe so it also covers entries with no rootns binding. Fixes: 197b6f6 ("Provide runtime veth config through control interface") Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
container_config_get_net_ifaces_list_new() builds a local mac_whitelist of mem_new()'d uint8_t * MAC entries and hands it to container_pnet_cfg_new(). The new() function deep-copies each entry into its own list rather than taking ownership, so the local list (both the list_t nodes and the MAC byte arrays) must be released by the caller. Currently the caller drops the list on the floor on every loop iteration where mac_filter is enabled, leaking one uint8_t * per whitelisted MAC plus a list_t node per entry. Walk and free the entries, then list_delete() the list, after every container_pnet_cfg_new() call (regardless of whether it returned a valid pnet_cfg). Fixes: 90274a3 ("daemon/container: propagate mac filter to c_net for physical netifs") Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.