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.

1 files changed, 5 insertions, 11 deletions

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)