From 3e62adfb214090de7ad531d3aa68570aae92f9ec Mon Sep 17 00:00:00 2001 From: Sergey Bugaev Date: Wed, 26 May 2021 16:30:53 +0300 Subject: proc: Drop some mach_port_destroy () uses mach_port_destroy () is a dangerous API that has to be used with extreme care. Namely, it destroys not one user reference, but *all* user references that a task has for a port name. Different parts of a program may all keep separate references on a port without coordinating it with each other (which is the whole idea behind reference counting). If one part of a program decides to destroy a port with mach_port_destroy () without informing others, others may still believe they hold a reference and will continue to use the name as if it still refered to the now-destroyed port right. This consitutes a port use-after-free, even if their use is also deallocating their reference. In the particular case of the proc server, this manifested itself as S_proc_reassign () destroying all user references to the task port right before the task port right is deallocated again in the dead-name notification handler. --- proc/mgt.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) (limited to 'proc') diff --git a/proc/mgt.c b/proc/mgt.c index d92bf528..929712f8 100644 --- a/proc/mgt.c +++ b/proc/mgt.c @@ -258,7 +258,7 @@ S_proc_reassign (struct proc *p, remove_proc_from_hash (p); task_terminate (p->p_task); - mach_port_destroy (mach_task_self (), p->p_task); + mach_port_deallocate (mach_task_self (), p->p_task); p->p_task = stubp->p_task; /* For security, we need to use the request port from STUBP */ @@ -934,18 +934,9 @@ process_has_exited (struct proc *p) /* No one is going to wait for processes in a task namespace. */ if (MACH_PORT_VALID (p->p_task_namespace)) { - mach_port_t task; mach_port_deallocate (mach_task_self (), p->p_task_namespace); p->p_waited = 1; - - /* XXX: `complete_exit' will destroy p->p_task if it is valid. - Prevent this so that `do_mach_notify_dead_name' can - deallocate the right. The proper fix is not to use - mach_port_destroy in the first place. */ - task = p->p_task; - p->p_task = MACH_PORT_NULL; complete_exit (p); - mach_port_deallocate (mach_task_self (), task); } } @@ -957,7 +948,10 @@ complete_exit (struct proc *p) remove_proc_from_hash (p); if (p->p_task != MACH_PORT_NULL) - mach_port_destroy (mach_task_self (), p->p_task); + { + mach_port_deallocate (mach_task_self (), p->p_task); + p->p_task = MACH_PORT_NULL; + } /* Remove us from our parent's list of children. */ if (p->p_sib) -- cgit v1.2.3