Hello, Thank you for your comments. Here is my attempt to correct these parts. Thanks,
-- Mikhail Karpov
From 1a90c9b6019c26c24b25576c00623f76be199e57 Mon Sep 17 00:00:00 2001 From: Mikhail Karpov <[email protected]> Date: Wed, 28 Oct 2026 13:19:08 +0700 Subject: [PATCH 2/2] Added checks for mach_port_mod_refs return value in several place --- console/console.c | 10 ++++- libdiskfs/peropen-make.c | 13 ++++++- libfshelp/translator-list.c | 21 +++++++++-- libnetfs/make-peropen.c | 13 ++++++- libshouldbeinlibc/exec-reauth.c | 35 +++++++++-------- libshouldbeinlibc/portxlate.c | 66 ++++++++++++++++----------------- libstore/unknown.c | 19 ++++++---- proc/host.c | 38 +++++++++++++++---- proc/mgt.c | 52 ++++++++++++++++++++------ proc/pgrp.c | 17 +++++++-- 10 files changed, 198 insertions(+), 86 deletions(-) diff --git a/console/console.c b/console/console.c index efc83da..f6b809d 100644 --- a/console/console.c +++ b/console/console.c @@ -1234,6 +1234,7 @@ netfs_S_io_map (struct protid *cred, flags = cred->po->openstat & (O_READ | O_WRITE); + error_t err; pthread_mutex_lock (&np->lock); switch (flags) { @@ -1242,7 +1243,14 @@ netfs_S_io_map (struct protid *cred, VM_PROT_READ | VM_PROT_WRITE); if (*wrobj == MACH_PORT_NULL) goto error; - mach_port_mod_refs (mach_task_self (), *rdobj, MACH_PORT_RIGHT_SEND, 1); + err = mach_port_mod_refs (mach_task_self (), *rdobj, + MACH_PORT_RIGHT_SEND, 1); + if (err) + { + pthread_mutex_unlock (&np->lock); + mach_port_deallocate (mach_task_self (), *rdobj); + return err; + } break; case O_READ: *rdobj = display_get_filemap (vcons->display, VM_PROT_READ); diff --git a/libdiskfs/peropen-make.c b/libdiskfs/peropen-make.c index 8eb0ada..6a29a99 100644 --- a/libdiskfs/peropen-make.c +++ b/libdiskfs/peropen-make.c @@ -72,8 +72,17 @@ diskfs_make_peropen (struct node *np, int flags, struct peropen *context, po->shadow_root_parent = context->shadow_root_parent; if (po->shadow_root_parent != MACH_PORT_NULL) - mach_port_mod_refs (mach_task_self (), po->shadow_root_parent, - MACH_PORT_RIGHT_SEND, 1); + { + err = mach_port_mod_refs (mach_task_self (), po->shadow_root_parent, + MACH_PORT_RIGHT_SEND, 1); + if (err) + { + if (po->shadow_root) + diskfs_nrele (po->shadow_root); + + goto free_path; + } + } } else { diff --git a/libfshelp/translator-list.c b/libfshelp/translator-list.c index 6d4f941..ab62064 100644 --- a/libfshelp/translator-list.c +++ b/libfshelp/translator-list.c @@ -129,13 +129,30 @@ fshelp_set_active_translator (struct port_info *pi, if (MACH_PORT_VALID (transbox->active) && transbox->active != t->active) { mach_port_t old; - err = mach_port_request_notification (mach_task_self (), transbox->active, + err = mach_port_request_notification (mach_task_self (), + transbox->active, MACH_NOTIFY_DEAD_NAME, 0, pi->port_right, MACH_MSG_TYPE_MAKE_SEND_ONCE, &old); if (err) goto out; + + err = mach_port_mod_refs (mach_task_self (), transbox->active, + MACH_PORT_RIGHT_SEND, +1); + if (err) + { + error_t request_err = + mach_port_request_notification (mach_task_self (), + transbox->active, + MACH_NOTIFY_DEAD_NAME, 0, + MACH_PORT_NULL, + MACH_MSG_TYPE_MAKE_SEND_ONCE, + &old); + assert_perror_backtrace (request_err); + goto out; + } + if (MACH_PORT_VALID (old)) mach_port_deallocate (mach_task_self (), old); @@ -147,8 +164,6 @@ fshelp_set_active_translator (struct port_info *pi, if (MACH_PORT_VALID (t->active)) mach_port_deallocate (mach_task_self (), t->active); - mach_port_mod_refs (mach_task_self (), transbox->active, - MACH_PORT_RIGHT_SEND, +1); t->active = transbox->active; } else if (! MACH_PORT_VALID (transbox->active)) diff --git a/libnetfs/make-peropen.c b/libnetfs/make-peropen.c index 37395e7..07d02d7 100644 --- a/libnetfs/make-peropen.c +++ b/libnetfs/make-peropen.c @@ -72,8 +72,17 @@ netfs_make_peropen (struct node *np, int flags, struct peropen *context) po->shadow_root_parent = context->shadow_root_parent; if (po->shadow_root_parent != MACH_PORT_NULL) - mach_port_mod_refs (mach_task_self (), po->shadow_root_parent, - MACH_PORT_RIGHT_SEND, 1); + { + err = mach_port_mod_refs (mach_task_self (), po->shadow_root_parent, + MACH_PORT_RIGHT_SEND, 1); + if (err) + { + if (po->shadow_root) + netfs_nrele (po->shadow_root); + + goto free_path; + } + } } netfs_nref (np); diff --git a/libshouldbeinlibc/exec-reauth.c b/libshouldbeinlibc/exec-reauth.c index b830f22..85b5a5d 100644 --- a/libshouldbeinlibc/exec-reauth.c +++ b/libshouldbeinlibc/exec-reauth.c @@ -23,6 +23,7 @@ #include <hurd/auth.h> #include <hurd/io.h> #include <hurd/process.h> +#include <assert-backtrace.h> /* Re-authenticates the ports in PORTS and FDS appropriately (replacing PORTS[INIT_PORT_AUTH] with AUTH) for a following exec using the auth port @@ -59,25 +60,27 @@ exec_reauth (auth_t auth, int secure, int must_reauth, if (!err) err = auth_user_authenticate (auth, ref, MACH_MSG_TYPE_MAKE_SEND, &newport); - mach_port_mod_refs (mach_task_self (), ref, MACH_PORT_RIGHT_RECEIVE, -1); - /* If they gave us nothing, make sure not to proceed. */ - if (!err && newport == MACH_PORT_NULL) - err = KERN_INVALID_ARGUMENT; + error_t refs_err = mach_port_mod_refs (mach_task_self (), ref, + MACH_PORT_RIGHT_RECEIVE, -1); + assert_perror_backtrace (refs_err); + /* If they gave us nothing, make sure not to proceed. */ + if (!err && newport == MACH_PORT_NULL) + err = KERN_INVALID_ARGUMENT; if (err) - return must_reauth ? err : 0; + return must_reauth ? err : 0; - if (isproc) - { - err = proc_reauthenticate_complete (newport); - if (err) - { - mach_port_deallocate (mach_task_self (), newport); - return must_reauth ? err : 0; - } - } + if (isproc) + { + err = proc_reauthenticate_complete (newport); + if (err) + { + mach_port_deallocate (mach_task_self (), newport); + return must_reauth ? err : 0; + } + } - mach_port_deallocate (mach_task_self (), *port); - *port = newport; + mach_port_deallocate (mach_task_self (), *port); + *port = newport; } return 0; } diff --git a/libshouldbeinlibc/portxlate.c b/libshouldbeinlibc/portxlate.c index eb6abfb..a022732 100644 --- a/libshouldbeinlibc/portxlate.c +++ b/libshouldbeinlibc/portxlate.c @@ -30,55 +30,55 @@ port_name_xlator_create (mach_port_t from_task, mach_port_t to_task, struct port_name_xlator **xlator) { error_t err; - struct port_name_xlator *x = malloc (sizeof (struct port_name_xlator)); + struct port_name_xlator *x = calloc (1, sizeof (struct port_name_xlator)); if (! x) - return ENOMEM; + return errno; + + err = mach_port_mod_refs (mach_task_self (), from_task, + MACH_PORT_RIGHT_SEND, +1); + if (err) + goto error; - mach_port_mod_refs (mach_task_self (), from_task, MACH_PORT_RIGHT_SEND, +1); x->from_task = from_task; - mach_port_mod_refs (mach_task_self (), to_task, MACH_PORT_RIGHT_SEND, +1); + err = mach_port_mod_refs (mach_task_self (), to_task, + MACH_PORT_RIGHT_SEND, +1); + if (err) + goto deallocate_from_task; + x->to_task = to_task; - x->to_names = 0; - x->to_types = 0; - x->to_names_len = 0; - x->to_types_len = 0; /* Cache a list of names in TO_TASK. */ err = mach_port_names (to_task, &x->to_names, &x->to_names_len, &x->to_types, &x->to_types_len); + if (err) + goto deallocate_to_task; - if (! err) - /* Make an array to hold ports from TO_TASK which have been translated - into our namespace. */ + /* Make an array to hold ports from TO_TASK which have been translated + into our namespace. */ + x->ports = calloc (1, sizeof (mach_port_t) * x->to_names_len); + if (!x->ports) { - x->ports = malloc (sizeof (mach_port_t) * x->to_names_len); - if (x->ports) - { - unsigned int i; - for (i = 0; i < x->to_names_len; i++) - x->ports[i] = MACH_PORT_NULL; - } - else - { - munmap ((caddr_t) x->to_names, - x->to_names_len * sizeof (mach_port_t)); - munmap ((caddr_t) x->to_types, - x->to_types_len * sizeof (mach_port_type_t)); + err = errno; + munmap ((caddr_t) x->to_names, + x->to_names_len * sizeof (mach_port_t)); + munmap ((caddr_t) x->to_types, + x->to_types_len * sizeof (mach_port_type_t)); + goto deallocate_to_task; + } - mach_port_deallocate (mach_task_self (), x->to_task); - mach_port_deallocate (mach_task_self (), x->from_task); + *xlator = x; + return 0; - err = ENOMEM; - } - } + deallocate_to_task: + mach_port_deallocate (mach_task_self (), to_task); - if (err) - free (x); - else - *xlator = x; + deallocate_from_task: + mach_port_deallocate (mach_task_self (), from_task); + error: + free (x); return err; } diff --git a/libstore/unknown.c b/libstore/unknown.c index 624c184..ce8eb31 100644 --- a/libstore/unknown.c +++ b/libstore/unknown.c @@ -60,10 +60,7 @@ noflags (struct store *store, int flags) static struct store_enc * duplicate_encoding (struct store_enc *enc) { - struct store_enc *us; - size_t i; - - us = calloc (1, sizeof *us); + struct store_enc *us = calloc (1, sizeof *us); if (us == NULL) return NULL; @@ -98,9 +95,17 @@ duplicate_encoding (struct store_enc *enc) us->num_offsets = enc->num_offsets; us->data_len = enc->data_len; - for (i = 0; i < us->num_ports; ++i) - mach_port_mod_refs (mach_task_self (), us->ports[i], - MACH_PORT_RIGHT_SEND, +1); + error_t err; + for (size_t i = 0; i < us->num_ports; ++i) + { + err = mach_port_mod_refs (mach_task_self (), us->ports[i], + MACH_PORT_RIGHT_SEND, +1); + if (err) + { + memset (&us->ports[i], 0, us->num_ports - i); + goto no_memory; + } + } return us; } diff --git a/proc/host.c b/proc/host.c index adda74d..58287b2 100644 --- a/proc/host.c +++ b/proc/host.c @@ -139,7 +139,6 @@ S_proc_getexecdata (struct proc *p, int **ints, mach_msg_type_number_t *nints) { - int i; int ports_allocated = 0; /* No need to check P here; we don't use it. */ @@ -151,31 +150,54 @@ S_proc_getexecdata (struct proc *p, *ports = mmap (0, round_page (n_std_ports * sizeof (mach_port_t)), PROT_READ|PROT_WRITE, MAP_ANON, 0, 0); if (*ports == MAP_FAILED) - return ENOMEM; + return errno; ports_allocated = 1; } memcpy (*ports, std_port_array, n_std_ports * sizeof (mach_port_t)); *nports = n_std_ports; + error_t err; + int ints_allocated = 0; if (*nints < n_std_ints) { *ints = mmap (0, round_page (n_std_ints * sizeof (int)), PROT_READ|PROT_WRITE, MAP_ANON, 0, 0); if (*ints == MAP_FAILED) { - if (ports_allocated) - munmap (*ports, round_page (n_std_ports * sizeof (mach_port_t))); - return ENOMEM; + err = errno; + goto munmap_ports; } + + ints_allocated = 1; } memcpy (*ints, std_int_array, n_std_ints * sizeof (int)); *nints = n_std_ints; - for (i = 0; i < n_std_ports; i++) - mach_port_mod_refs (mach_task_self (), std_port_array[i], MACH_PORT_RIGHT_SEND, 1); - *portspoly = MACH_MSG_TYPE_MOVE_SEND; + for (int i = 0; i < n_std_ports; ++i) + { + err = mach_port_mod_refs (mach_task_self (), std_port_array[i], + MACH_PORT_RIGHT_SEND, 1); + if (err) + { + for (int j = 0; j < i; ++j) + mach_port_deallocate (mach_task_self (), std_port_array[j]); + + goto munmap_ints; + } + } + *portspoly = MACH_MSG_TYPE_MOVE_SEND; return 0; + + munmap_ints: + if (ints_allocated) + munmap (*ints, round_page (n_std_ints * sizeof (int))); + + munmap_ports: + if (ports_allocated) + munmap (*ports, round_page (n_std_ports * sizeof (mach_port_t))); + + return err; } /* Implement proc_execdata_notify as described in <hurd/process.defs>. */ diff --git a/proc/mgt.c b/proc/mgt.c index 8223bf5..c898d0d 100644 --- a/proc/mgt.c +++ b/proc/mgt.c @@ -221,11 +221,33 @@ S_proc_child (struct proc *parentp, if (childp->p_parentset) return EBUSY; + int allocate_exe = 0; if (!childp->exe && parentp->exe) { childp->exe = strdup (parentp->exe); if (!childp->exe) - return errno; + return errno; + + allocate_exe = 1; + } + + if (MACH_PORT_VALID (parentp->p_task_namespace)) + { + error_t err = mach_port_mod_refs (mach_task_self (), + parentp->p_task_namespace, + MACH_PORT_RIGHT_SEND, +1); + if (err) + { + if (allocate_exe) + { + free (childp->exe); + childp->exe = NULL; + } + + return err; + } + + childp->p_task_namespace = parentp->p_task_namespace; } mach_port_deallocate (mach_task_self (), childt); @@ -281,13 +303,6 @@ S_proc_child (struct proc *parentp, childp->end_code = parentp->end_code; } - if (MACH_PORT_VALID (parentp->p_task_namespace)) - { - mach_port_mod_refs (mach_task_self (), parentp->p_task_namespace, - MACH_PORT_RIGHT_SEND, +1); - childp->p_task_namespace = parentp->p_task_namespace; - } - return 0; } @@ -1386,19 +1401,34 @@ S_mach_notify_new_task (struct port_info *notify, if (! parentp) return ESRCH; + error_t err; + int needs_task_dealloc = 0; childp = task_find_nocreate (task); if (! childp) { - mach_port_mod_refs (mach_task_self (), task, MACH_PORT_RIGHT_SEND, +1); + err = mach_port_mod_refs (mach_task_self (), task, + MACH_PORT_RIGHT_SEND, +1); + if (err) + return err; + childp = new_proc (task); + needs_task_dealloc = 1; } if (MACH_PORT_VALID (parentp->p_task_namespace)) { - error_t err; /* Tasks in a task namespace are not expected to call proc_child, so we do it on their behalf. */ - mach_port_mod_refs (mach_task_self (), task, MACH_PORT_RIGHT_SEND, +1); + err = mach_port_mod_refs (mach_task_self (), task, + MACH_PORT_RIGHT_SEND, +1); + if (err) + { + if (needs_task_dealloc) + mach_port_deallocate (mach_task_self (), task); + + return err; + } + err = S_proc_child (parentp, task); assert_perror_backtrace (err); diff --git a/proc/pgrp.c b/proc/pgrp.c index cf64697..7bce025 100644 --- a/proc/pgrp.c +++ b/proc/pgrp.c @@ -117,10 +117,21 @@ S_proc_setsid (struct proc *p) if (p->p_pgrp->pg_pgid == p->p_pid || pgrp_find (p->p_pid)) return EPERM; - leave_pgrp (p); - sess = new_session (p); - p->p_pgrp= new_pgrp (p->p_pid, sess); + if (!sess) + return errno; + + struct pgrp *pgrp = new_pgrp (p->p_pid, sess); + if (!pgrp) + { + error_t err = errno; + remove_session_from_hash (sess); + free (sess); + return err; + } + + leave_pgrp (p); + p->p_pgrp = pgrp; join_pgrp (p); return 0; -- 2.43.0
