Leak fixes#596
Open
quitschbo wants to merge 30 commits into
Open
Conversation
quitschbo
force-pushed
the
leak-fixes
branch
10 times, most recently
from
0051184 to
ec71b64
Compare
quitschbo
force-pushed
the
leak-fixes
branch
3 times, most recently
from
9b310e2 to
c5732e2
Compare
INIT is a thin wrapper for __attribute__((constructor)) and is used throughout the daemon to register module-level setup code that must run before main(). There is no symmetric helper for the matching __attribute__((destructor)) case. Add DEINIT next to INIT for module-level teardown that should fire at exit() time without a runtime registration step (atexit() etc.). Add DEINIT_LAST as a destructor pinned to priority 101 (the lowest user-allowed priority) so the function fires last among all destructor handlers, useful for teardown that must outlive every other DEINIT (e.g. closing log handlers used by the rest of the destructor chain). The first users follow in subsequent commits. Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
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 a paired unit_unregister_compartment_module() that list_remove()s the module from the list. Each u_* module gets a matching DEINIT-marked u_*_deinit() that calls unregister, so the registration and the corresponding cleanup live next to each other in the same module file. Once every module has unregistered at destructor time the list head is NULL and the list_t nodes are gone with no central sweep needed. 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>
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. Fold the event teardown into main_deinit() alongside the pre-existing logf teardown (formerly main_logf_cleanup) by calling event_reset(), which drains event_timer_list, event_signal_list and event_inotify_list and frees each entry via the existing wrapped_remove_* helpers. This avoids having to track main's three event handles via file-scope statics and as a side effect also catches any subsystem that forgot to clean up its own event entries. Tag main_deinit() with DEINIT_LAST so it runs last among all DEINIT handlers, preserving the "logf teardown happens last" guarantee that the old INIT-time atexit() registration provided. By that time every atexit cleanup (cmld_cleanup, scd_cleanup, audit_cleanup, ...) has already pulled its own event handles off the lists, so event_reset() just sweeps the residual entries. 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), guarded by the explicit NULL check used elsewhere in container_free() for optional char* fields (cpus_allowed, dns_server, etc.). 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>
mount_entry_set_verity_sha256() mem_strdup()s the verity root hash into mntent->verity_sha256, but mount_free() only released image_file, mount_point, fs_type, sha1, sha256 and mount_data and did not touch the verity_sha256 field. Every dm-verity backed guestos image therefore leaked one mem_strdup on every mount_free() call. ASan attributed the strdups to guestos_config_fill_mount_internal() callers including guestos_mgr_get_latest_by_name(), guestos_images_are_complete() (from guestos_mgr_purge_obsolete) and c_vol_new(), all of which build temporary mount tables that get released via mount_free(). Release verity_sha256 alongside the other optional char* fields. Fixes: 1a3840d ("daemon: introduced dm-verity for guestos images") Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
u_idmapped_add_mount() appends a freshly-allocated u_idmapped_mnt_t (struct + two mem_strdup'd paths) to idmapped->mnt_list for every mount the unit is configured with (e.g. for SCD: /run/socket, /data/logs, /data/cml/tokens). u_idmapped_free() released only the u_idmapped_t wrapper, leaving every mount entry, both strdups and the list_t nodes leaked on every compartment teardown. Walk mnt_list and call u_idmapped_mnt_free() per entry, then list_delete() the list before freeing the wrapper. Fixes: 5b3602e ("daemon/u_idmapped: Introduce u_idmapped module") Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
cmld_reload_container_internal() builds a local uuid_t via uuid_new(uuid_string(uuid)) at the top, passes it by const pointer to cmld_container_new() (which deep-copies into its own uuid), and released the local on the cleanup path with mem_free0(uuid_tmp). That only freed the uuid_t struct itself, not the mem_strdup'd uuid->string inside, leaking ~37 B per reload via uuid_new() at uuid.c:135. Switch to uuid_free(), which mem_free0()s both string and struct. Fixes: 3a72fa9 ("daemon: Reload container on config update") Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
scd_init() builds the scd unit with scd_unit = unit_new(uuid_new(SCD_UUID), ...); unit_new() takes a const uuid_t * and compartment_new() makes its own copy via uuid_new(uuid_string(uuid)), so the inline uuid_new() return is leaked after unit_new() returns: ~32 B (uuid_t struct) + ~37 B (uuid->string strdup) per cmld start. Capture the uuid in a local, pass it to unit_new(), and uuid_free() it afterwards. uuid_free() is NULL-safe, so this is also correct on unit_new() failure paths. Fixes: a002239 ("daemon/scd: start unit of scd in private netns") Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
tss_init() builds the tpm2d unit with tpm2d_unit = unit_new(uuid_new(TPM2D_UUID), ...); unit_new() takes a const uuid_t * and compartment_new() makes its own copy via uuid_new(uuid_string(uuid)), so the inline uuid_new() return is leaked after unit_new() returns whenever a TPM is present and tss is initialised: ~32 B (uuid_t struct) + ~37 B (uuid->string strdup). Capture the uuid in a local, pass it to unit_new(), and uuid_free() it afterwards. uuid_free() is NULL-safe, so this is also correct on unit_new() failure paths. Fixes: 3200a6f ("daemon/tss: start tpm2d as unit with auto reconnect") Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
c_cgroups_dev_new() builds cgroups_dev->path via mem_printf() (the per-container cgroup subtree path used throughout the module), but the .compartment_free hook only released the wrapper struct itself, leaking one mem_printf result per container teardown. c_cgroups_dev_cleanup() (.cleanup hook) already drains the assigned/allowed/denied device lists and the BPF program on stop, but path lives across stop/start cycles so it belongs in the free hook. Add the missing mem_free0(cgroups_dev->path). Fixes: 484f0e0 ("daemon/c_cgroups_dev: container callbacks for device allow and deny") Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
audit_init() allocates an nl_sock_t for NETLINK_AUDIT, registers cmld as the kernel auditd, and attaches an event_io_t to read incoming audit records. Both were stored in function-local variables and left in place for the daemon's lifetime, leaking ~24 B (event_io) + ~16 B (nl_sock internal) at exit. Promote audit_sock and audit_io_event to file-scope statics and add audit_cleanup() that event_remove_io()/event_io_free()s the io and nl_sock_free()s the socket. Register it via atexit() in cmld_init_stage_unit() once audit_init() succeeds, matching the pattern used by the other cmld-initialized subsystems (cmld_cleanup, scd_cleanup, hotplug_cleanup, lxcfs_cleanup, guestos_mgr_cleanup). Fixes: dbb6194 ("daemon/audit: Introduce kernel audit handling") Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
scd_on_connect_cb() registers an event_io_t to read TokenToDaemon events from scd. The runtime disconnect path in scd_event_cb_recv_message() does event_remove_io()/event_io_free() on errors, but if cmld exits while still connected the event_io_t and its 24 B list_t are leaked. Promote the event handle to a file-scope static scd_event_io. Clear it in the disconnect path so the cleanup function does not double free, and have scd_cleanup() event_remove_io() + event_io_free() it when still live on exit. Fixes: 277e7e1 ("daemon/scd: handle scd SE_REMOVED event") Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
control_new() created an event_io_t to handle accept() on the listening socket and added it to the event loop without storing the handle anywhere. control_free() only cleaned up the per-client event_io_sock_connected_list, so the accept-side io and its 24 B list_t in event_io_list were leaked at cmld exit. Store the handle in control->event_io_accept, and have control_free() event_remove_io() + event_io_free() it alongside the connected client list cleanup. 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>
c_vol_cleanup_dm() mem_printf()s a per-entry label (the "<uuid>-<image>" device-mapper name) before querying the dm target type. The normal path mem_free0()s label at the bottom of the loop, but the early `continue` taken when dm_get_target_type_new() returns NULL skips that free, leaking the label string on every cleanup pass where a dm device cannot be inspected (e.g. tmpfs-only entries that have no device-mapper backing, as ASan reports for the 00000000-...-tmpfs entry of c0). Free label before the continue. Fixes: 1a3840d ("daemon: introduced dm-verity for guestos images") Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
container.c keeps a static list of registered container compartment modules. Modules are registered at INIT time via container_register_compartment_module() (c_hotplug, c_smartcard, c_user, c_idmapped, c_cgroups_v2, c_cgroups_dev, c_net, c_vol, c_service, c_run, c_fifo, c_time, c_audit, c_cap, c_seccomp, c_automount, c_xorg_compat, c_oci, c_cgroups_sockopt) and the list_t nodes were never freed at shutdown. In parallel, the per-handler wrapper structs allocated by CONTAINER_MODULE_REGISTER_WRAPPER_IMPL (~40 handlers across c_user, c_smartcard, c_cgroups_v2, c_net, c_vol, c_audit, c_run, c_time, c_cap, c_idmapped, c_service, c_cgroups_dev) were also never released. Add paired unregister APIs: - container_unregister_compartment_module() list_remove()s a module from the global module list. - The CONTAINER_MODULE_REGISTER_WRAPPER_IMPL macro now also emits a container_unregister_<name>_handler() that frees the per-handler wrapper struct. Each c_* module gets a matching DEINIT-marked c_*_deinit() that calls the corresponding unregisters for both the module and any handlers it registered, so the registration and the corresponding cleanup live next to each other in the same module file. For modules that already had a deinit doing other work (c_cgroups, c_cgroups_dev, c_cgroups_v2, c_fifo, c_hotplug), fold the unregisters into the existing deinit and convert that function from a runtime atexit() registration into a DEINIT attribute, dropping the WARN-on-atexit-failure boilerplate. Once every module has unregistered the list head is NULL, the handler structs are gone, and no central sweep is needed. Fixes: 7ccd51d ("daemon: moved compartment_module list to container module") 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>
Fixed following typos: s/registerd/registered/, s/allready/already/ Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
container.h hand-wrote the function, register, and unregister declarations for the get_vnet_runtime_cfg_new container handler instead of using CONTAINER_MODULE_WRAPPER_DECLARE() like every other handler in the file. The implementation side (container.c) already uses CONTAINER_MODULE_REGISTER_WRAPPER_IMPL and CONTAINER_MODULE_FUNCTION_WRAPPER_IMPL, so the asymmetry was only on the header side and caused the unregister declaration to be missed when the macro was extended to also emit container_unregister_*_handler() prototypes. Replace the three hand-written declarations with a single CONTAINER_MODULE_WRAPPER_DECLARE(get_vnet_runtime_cfg_new, list_t *) in the section that already collects the other container module wrapper declarations. Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
…fy_buf_cb push_config_verify_buf_cb() borrowed os_name via guestos_get_name(os), which returns a pointer into the protobuf-unpacked config owned by os. Any error path falling through cleanup_os calls guestos_free() which releases those strings, so the audit_log_event() at err reads a dangling pointer. ASan flagged this as a heap-use-after-free. Duplicate os_name into a heap-owned local right after guestos_new_from_buffer() so it survives guestos_free(). When os is NULL, strdup the placeholder "NA" so the audit event still carries a valid subject. Release the copy on both exit paths. Fixes: 7a35942 ("daemon/audit: Generate audit messages") Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
file_disk_space_available() only logged "Not enough disk space left" without the actual numbers. Print the required and available byte counts as well as the thrashold and internaly computed min_free_space. alongside the error. Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
The shared data directory under DEFAULT_BASE_PATH/shared is used to hand files between cmld and the units it spawns (in particular SCD), which run as different UIDs after uid mapping. Creating the directory mode 0700 blocks non-root readers, and if the directory already exists from a previous run its permissions are left as-is. Create the directory with mode 0755 and, if it already exists, chmod(0755) it so the permissions are enforced on every start. Fail fatally on either error. The directory itself is older than this issue, but the readability requirement only appeared once cmld started spawning units in their own user namespaces. Fixes: fb6de43 ("daemon/unit: Introduce new unit module for minimal compartments") Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
write_to_tmpfile_new() creates a temp file under DEFAULT_BASE_PATH/shared so cmld can hand it to the SCD unit for hashing (e.g. newca_certs). mkstemp() creates files mode 0600, which is unreadable from SCD after uid mapping. fchmod() the descriptor to 0644 immediately after mkstemp(). On chmod failure, close the fd, unlink the just-created file and return NULL so the caller sees a clean error. Also drop the stale "FIXME: move SCD in own container" comment — SCD already runs in its own unit, and use of the shared directory is the established mechanism. The shared-dir path pre-existed, but the readability requirement across a distinct UID only became a problem once SCD moved into a unit with its own user namespace. Fixes: 8b63acc ("daemon/scd: start scd as unit with auto reconnect") Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
On the os_ver == old_ver retrigger path of push_config_verify_buf_cb() the freshly unpacked guestos_t duplicate jumped straight to trigger_download without ever being appended to guestos_list, so the whole protobuf-unpacked config object was orphaned on every repeated push of an already installed version. Free the duplicate os before the retrigger goto. No files have been written yet on that path, so plain guestos_free() instead of guestos_purge() is sufficient, and the audit/log strings keep working since os_name is a heap-owned copy. The error labels remain the sole cleanup for their paths, so nothing is freed twice. Fixes: 74751d2 ("daemon/guestos_mgr: Proper image download and failed download handling") Assisted-by: Claude:claude-fable-5 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
The mount_t built in push_config_verify_buf_cb() for the disk space check (mount_new() + guestos_fill_mount()/guestos_fill_mount_setup()) was only released via the cleanup_mnt error label. Every push that passed signature verification and the disk space check therefore leaked the whole mount table including its strdup'd entries — on fresh installs, updates and same-version re-pushes alike. mount_free(mnt) at trigger_download, which is reached by all success paths. The error labels remain the sole cleanup for their paths, so nothing is freed twice. Fixes: 3c8fd49 ("daemon/guestos_mgr: Check available disk space") Assisted-by: Claude:claude-fable-5 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
scd_on_connect_cb() overwrites the file-scope scd_event_io unconditionally on every (re-)connect of the scd unit. If the new connection is established while the previous connection's event io is still registered, i.e. its EVENT_IO_EXCEPT error event has not been processed yet, the old handle is untracked from that point on. It is still freed if its error event fires later, but if cmld exits first, scd_cleanup() only releases the new handle and the old event_io_t, its list_t node and the connected socket fd are leaked. Release a still-set scd_event_io before creating the new one: event_remove_io()/event_io_free() the handle and close its socket fd, mirroring the connection_err path of scd_event_cb_recv_message(). Fixes: 8b63acc ("daemon/scd: start scd as unit with auto reconnect") Assisted-by: Claude:claude-fable-5 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
scd_cleanup() releases the scd event io via event_remove_io()/event_io_free() but never closes the underlying connected socket fd, unlike the runtime disconnect path in scd_event_cb_recv_message() which does close(fd) after freeing the io. Since scd_cleanup() runs at exit the kernel reclaims the descriptor anyway, but the teardown should mirror the disconnect path. Grab the fd via event_io_get_fd() before freeing the io and close it. Fixes: 21516ec ("daemon/scd: Release scd connection event_io at exit") Assisted-by: Claude:claude-fable-5 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.
This series fixes a collection of memory leaks in cmld (mostly found with ASan), introduces infrastructure for clean
module teardown at exit, and contains two small functional fixes needed since SCD runs as a unit with its own user
namespace.
Teardown infrastructure
(attribute((destructor)), with DEINIT_LAST pinned to the lowest priority so it fires after all other
destructors).
CONTAINER_MODULE_REGISTER_WRAPPER_IMPL now emits a matching container_unregister_handler(). Every c / u_* module
gets a DEINIT-marked deinit that unregisters what it registered, so registration and cleanup live next to each
other and no central sweep is needed.
teardown happens last" guarantee.
Runtime memory-leak fixes (accumulate during normal operation)
deep-copies it.
frees the dm label on the early-continue path.
mount_t on all success paths.
Exit-time releases
additionally releases a stale event io left over from a previous connection before overwriting the handle, and
scd_cleanup() closes the underlying socket fd to mirror the runtime disconnect path.
cmld_reload_container_internal() (previous mem_free0() leaked the inner string).
Bug fix
guestos_free() released before the audit log on error paths read it (flagged by ASan). The name is now duplicated
into a heap-owned local.
Functional fixes for SCD as unit
write_to_tmpfile_new() chmods its output to 0644, so the SCD unit can read files handed over by cmld across the uid mapping. Only public CA certificates are placed there; owner-only write is preserved.
Misc
CONTAINER_MODULE_WRAPPER_DECLARE() like every other handler.