Skip to content

Commit

Permalink
darwin: Fix racy leakage of memory during teardown
Browse files Browse the repository at this point in the history 
Where DarwinHelperBackend.close() would cancel dispatch sources, but
return before the cancel handler got a chance to run. While this leak
typically happened on teardown and wasn't necessarily a big deal
locally, the worse part was that we also leaked large regions of memory
in remote processes that we uninjected from just before teardown.
  • Loading branch information
@oleavr
oleavr committed Feb 28, 2025
1 parent c537624 commit 60a6770
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 25 deletions.
36 changes: 23 additions & 13 deletions src/darwin/frida-helper-backend-glue.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@
};

static FridaSpawnInstance * frida_spawn_instance_new (FridaDarwinHelperBackend * backend);
static void frida_spawn_instance_free (FridaSpawnInstance * instance);
static void frida_spawn_instance_close (FridaSpawnInstance * instance);
static void frida_spawn_instance_resume (FridaSpawnInstance * self);

static void frida_spawn_instance_on_server_cancel (void * context);
Expand Down Expand Up @@ -410,7 +410,7 @@

static FridaInjectInstance * frida_inject_instance_new (FridaDarwinHelperBackend * backend, guint id, guint pid);
static FridaInjectInstance * frida_inject_instance_clone (const FridaInjectInstance * instance, guint id);
static void frida_inject_instance_free (FridaInjectInstance * instance);
static void frida_inject_instance_close (FridaInjectInstance * instance);
static gboolean frida_inject_instance_task_did_not_exec (FridaInjectInstance * instance);

static gboolean frida_inject_instance_start_thread (FridaInjectInstance * self, GError ** error);
Expand Down Expand Up @@ -934,7 +934,7 @@ static kern_return_t frida_convert_thread_state (mach_port_t thread, FridaConver
{
if (instance->pid != 0)
kill (instance->pid, SIGKILL);
frida_spawn_instance_free (instance);
frida_spawn_instance_close (instance);

pid = 0;

Expand Down Expand Up @@ -2200,9 +2200,9 @@ static void frida_darwin_helper_backend_launch_using_lsaw (NSString * identifier
}

void
_frida_darwin_helper_backend_free_spawn_instance (FridaDarwinHelperBackend * self, void * instance)
_frida_darwin_helper_backend_close_spawn_instance (FridaDarwinHelperBackend * self, void * instance)
{
frida_spawn_instance_free (instance);
frida_spawn_instance_close (instance);
}

guint
Expand Down Expand Up @@ -2489,7 +2489,7 @@ static void frida_darwin_helper_backend_launch_using_lsaw (NSString * identifier
}
failure:
{
frida_inject_instance_free (instance);
frida_inject_instance_close (instance);
goto beach;
}
beach:
Expand Down Expand Up @@ -2646,7 +2646,7 @@ static void frida_darwin_helper_backend_launch_using_lsaw (NSString * identifier
FridaInjectInstance * self = context;

dispatch_release (g_steal_pointer (&self->thread_monitor_source));
frida_inject_instance_free (self);
frida_inject_instance_close (self);
}

static void
Expand Down Expand Up @@ -2738,9 +2738,9 @@ static void frida_darwin_helper_backend_launch_using_lsaw (NSString * identifier
}

void
_frida_darwin_helper_backend_free_inject_instance (FridaDarwinHelperBackend * self, void * instance)
_frida_darwin_helper_backend_close_inject_instance (FridaDarwinHelperBackend * self, void * instance)
{
frida_inject_instance_free (instance);
frida_inject_instance_close (instance);
}

static FridaSpawnInstance *
Expand All @@ -2750,7 +2750,7 @@ static void frida_darwin_helper_backend_launch_using_lsaw (NSString * identifier
guint i;

instance = g_slice_new0 (FridaSpawnInstance);
instance->backend = backend;
instance->backend = g_object_ref (backend);
instance->thread = MACH_PORT_NULL;

instance->server_port = MACH_PORT_NULL;
Expand All @@ -2773,11 +2773,13 @@ static void frida_darwin_helper_backend_launch_using_lsaw (NSString * identifier
instance->page_pool[i].scratch_page = 0;
}

_frida_darwin_helper_backend_on_instance_created (backend, instance);

return instance;
}

static void
frida_spawn_instance_free (FridaSpawnInstance * instance)
frida_spawn_instance_close (FridaSpawnInstance * instance)
{
task_t self_task;
FridaExceptionPortSet * previous_ports;
Expand Down Expand Up @@ -2816,6 +2818,9 @@ static void frida_darwin_helper_backend_launch_using_lsaw (NSString * identifier
if (instance->dyld != NULL)
g_object_unref (instance->dyld);

_frida_darwin_helper_backend_on_instance_destroyed (instance->backend, instance);
g_object_unref (instance->backend);

g_slice_free (FridaSpawnInstance, instance);
}

Expand Down Expand Up @@ -2858,7 +2863,7 @@ static void frida_darwin_helper_backend_launch_using_lsaw (NSString * identifier
FridaSpawnInstance * self = context;

dispatch_release (g_steal_pointer (&self->server_recv_source));
frida_spawn_instance_free (self);
frida_spawn_instance_close (self);
}

static void
Expand Down Expand Up @@ -4114,6 +4119,8 @@ static void frida_darwin_helper_backend_launch_using_lsaw (NSString * identifier

instance->backend = g_object_ref (backend);

_frida_darwin_helper_backend_on_instance_created (backend, instance);

return instance;
}

Expand Down Expand Up @@ -4143,11 +4150,13 @@ static void frida_darwin_helper_backend_launch_using_lsaw (NSString * identifier

g_object_ref (clone->backend);

_frida_darwin_helper_backend_on_instance_created (clone->backend, clone);

return clone;
}

static void
frida_inject_instance_free (FridaInjectInstance * instance)
frida_inject_instance_close (FridaInjectInstance * instance)
{
FridaAgentContext * agent_context = instance->agent_context;
task_t self_task;
Expand Down Expand Up @@ -4187,6 +4196,7 @@ static void frida_darwin_helper_backend_launch_using_lsaw (NSString * identifier
if (instance->task != MACH_PORT_NULL)
mach_port_deallocate (self_task, instance->task);

_frida_darwin_helper_backend_on_instance_destroyed (instance->backend, instance);
g_object_unref (instance->backend);

g_slice_free (FridaInjectInstance, instance);
Expand Down
49 changes: 37 additions & 12 deletions src/darwin/frida-helper-backend.vala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ namespace Frida {
private Gee.HashMap<void *, uint> inject_cleaner_by_instance = new Gee.HashMap<void *, uint> ();
private Gee.HashMap<uint, uint> inject_expiry_timers = new Gee.HashMap<uint, uint> ();

private Gee.Set<void *> instances = new Gee.HashSet<void *> ();
private SourceFunc? on_last_instance_destroyed = null;

public uint next_id = 1;

private PolicySoftener policy_softener;
Expand Down Expand Up @@ -65,10 +68,6 @@ namespace Frida {
}

~DarwinHelperBackend () {
foreach (var instance in spawn_instances.values)
_free_spawn_instance (instance);
foreach (var instance in inject_instances.values)
_free_inject_instance (instance);
_destroy_dispatch_context ();
}

Expand All @@ -79,11 +78,24 @@ namespace Frida {
pending.complete ();

foreach (var entry in inject_cleaner_by_instance.entries) {
_free_inject_instance (entry.key);
_close_inject_instance (entry.key);
Source.remove (entry.value);
}
inject_cleaner_by_instance.clear ();

foreach (var instance in spawn_instances.values)
_close_spawn_instance (instance);
spawn_instances.clear ();

bool any_pending = false;
lock (instances) {
any_pending = !instances.is_empty;
if (any_pending)
on_last_instance_destroyed = close.callback;
}
if (any_pending)
yield;

if (dtrace_agent != null) {
dtrace_agent.spawn_added.disconnect (on_dtrace_agent_spawn_added);
dtrace_agent.spawn_removed.disconnect (on_dtrace_agent_spawn_removed);
Expand Down Expand Up @@ -189,7 +201,7 @@ namespace Frida {
inject_cleaner_by_instance.unset (instance, out source_id);
Source.remove (source_id);

_free_inject_instance (instance);
_close_inject_instance (instance);
}

policy_softener.forget (pid);
Expand All @@ -207,7 +219,7 @@ namespace Frida {

void * instance;
if (spawn_instances.unset (pid, out instance))
_free_spawn_instance (instance);
_close_spawn_instance (instance);

child_dead (pid);
}
Expand Down Expand Up @@ -295,7 +307,7 @@ namespace Frida {
void * instance;
if (spawn_instances.unset (pid, out instance)) {
_resume_spawn_instance (instance);
_free_spawn_instance (instance);
_close_spawn_instance (instance);
} else {
resume_with_validation (pid);
}
Expand Down Expand Up @@ -414,7 +426,7 @@ namespace Frida {
} catch (GLib.Error e) {
if (instance_created_here) {
spawn_instances.unset (pid);
_free_spawn_instance (spawn_instance);
_close_spawn_instance (spawn_instance);
}

throw_api_error (e);
Expand Down Expand Up @@ -547,7 +559,7 @@ namespace Frida {
private void schedule_inject_instance_cleanup (void * instance) {
var cleanup_source = new TimeoutSource (50);
cleanup_source.set_callback (() => {
_free_inject_instance (instance);
_close_inject_instance (instance);

var removed = inject_cleaner_by_instance.unset (instance);
assert (removed);
Expand Down Expand Up @@ -603,6 +615,19 @@ namespace Frida {
policy_softener.forget (pid);
}

public void _on_instance_created (void * instance) {
lock (instances)
instances.add (instance);
}

public void _on_instance_destroyed (void * instance) {
lock (instances) {
instances.remove (instance);
if (on_last_instance_destroyed != null && instances.is_empty)
schedule_on_frida_thread ((owned) on_last_instance_destroyed);
}
}

private void on_dtrace_agent_spawn_added (HostSpawnInfo info) {
spawn_added (info);
}
Expand Down Expand Up @@ -650,15 +675,15 @@ namespace Frida {
protected extern void * _create_spawn_instance (uint pid);
protected extern void _prepare_spawn_instance_for_injection (void * instance, uint task) throws Error;
protected extern void _resume_spawn_instance (void * instance);
protected extern void _free_spawn_instance (void * instance);
protected extern void _close_spawn_instance (void * instance);

protected extern uint _inject_into_task (uint pid, uint task, string path_or_name, MappedLibraryBlob? blob, string entrypoint, string data) throws Error;
protected extern void _demonitor (void * instance);
protected extern uint _demonitor_and_clone_injectee_state (void * instance);
protected extern void _recreate_injectee_thread (void * instance, uint pid, uint task) throws Error;
protected extern void _join_inject_instance_posix_thread (void * instance, void * posix_thread);
protected extern uint _get_pid_of_inject_instance (void * instance);
protected extern void _free_inject_instance (void * instance);
protected extern void _close_inject_instance (void * instance);
}

public class DTraceAgent : Object {
Expand Down

0 comments on commit 60a6770

Please sign in to comment.