Hello, On Fri, 23 Oct 2026 01:19:36 Samuel Thibault wrote: > > Also, I’d like to know whether the return value of > > mach_port_mod_refs should be checked as well. > > Yes, we'd want to check that rather than letting refs miscounted.
Understood; I plan to look into this. > We'd also want to avoid doing the rest of the function. Actually the > function could be simplified first in a preparatory patch, with some > return err; rather than "else" statements. Agreed, it was possible to simplify this function. The code looks tidier now. Thanks, -- Mikhail Karpov
From 784b1f9f912573252a70ec412308e18942a98ff0 Mon Sep 17 00:00:00 2001 From: Mikhail Karpov <[email protected]> Date: Fri, 23 Oct 2026 20:40:29 +0700 Subject: [PATCH 2/3] Added checks for mach_port_insert_right return value in several place --- console/pager.c | 9 ++- eth-multiplexer/ethernet.c | 6 +- isofs/pager.c | 10 +++- libdiskfs/disk-pager.c | 6 +- libirqhelp/irqhelp.c | 14 +++-- lwip/port/netif/hurdethif.c | 111 ++++++++++++++++-------------------- pfinet/ethernet.c | 6 +- storeio/pager.c | 12 +++- term/users.c | 14 +++-- trans/streamio.c | 62 ++++++++++---------- trans/symlink.c | 7 ++- 11 files changed, 141 insertions(+), 116 deletions(-) diff --git a/console/pager.c b/console/pager.c index a2c9069..8499b82 100644 --- a/console/pager.c +++ b/console/pager.c @@ -167,8 +167,13 @@ user_pager_create (struct user_pager *user_pager, unsigned int npages, user_pager->memobj = pager_get_port (user_pager->pager); ports_port_deref (user_pager->pager); - mach_port_insert_right (mach_task_self (), user_pager->memobj, - user_pager->memobj, MACH_MSG_TYPE_MAKE_SEND); + err = mach_port_insert_right (mach_task_self (), user_pager->memobj, + user_pager->memobj, MACH_MSG_TYPE_MAKE_SEND); + if (err) + { + mach_port_deallocate (mach_task_self (), user_pager->memobj); + return err; + } *user = 0; err = vm_map (mach_task_self (), diff --git a/eth-multiplexer/ethernet.c b/eth-multiplexer/ethernet.c index 13f0036..129be7b 100644 --- a/eth-multiplexer/ethernet.c +++ b/eth-multiplexer/ethernet.c @@ -135,8 +135,10 @@ int ethernet_open (char *dev_name, device_t master_device, if (err) error (2, err, "ports_create_port"); readptname = ports_get_right (readpt); - mach_port_insert_right (mach_task_self (), readptname, readptname, - MACH_MSG_TYPE_MAKE_SEND); + err = mach_port_insert_right (mach_task_self (), readptname, readptname, + MACH_MSG_TYPE_MAKE_SEND); + if (err) + error (2, err, "mach_port_insert_right"); mach_port_set_qlimit (mach_task_self (), readptname, MACH_PORT_QLIMIT_MAX); diff --git a/isofs/pager.c b/isofs/pager.c index 621214c..7b36f36 100644 --- a/isofs/pager.c +++ b/isofs/pager.c @@ -214,8 +214,14 @@ diskfs_get_filemap (struct node *np, vm_prot_t prot) pthread_spin_unlock (&node2pagelock); - mach_port_insert_right (mach_task_self (), right, right, - MACH_MSG_TYPE_MAKE_SEND); + error_t err = mach_port_insert_right (mach_task_self (), right, right, + MACH_MSG_TYPE_MAKE_SEND); + if (err) + { + mach_port_deallocate (mach_task_self (), right); + errno = err; + return MACH_PORT_NULL; + } return right; } diff --git a/libdiskfs/disk-pager.c b/libdiskfs/disk-pager.c index 6eaa3c6..0a6076c 100644 --- a/libdiskfs/disk-pager.c +++ b/libdiskfs/disk-pager.c @@ -62,8 +62,10 @@ diskfs_start_disk_pager (struct user_pager_info *upi, /* Get a port to the disk pager. */ disk_pager_port = pager_get_port (diskfs_disk_pager); - mach_port_insert_right (mach_task_self (), disk_pager_port, disk_pager_port, - MACH_MSG_TYPE_MAKE_SEND); + err = mach_port_insert_right (mach_task_self (), disk_pager_port, + disk_pager_port, MACH_MSG_TYPE_MAKE_SEND); + if (err) + error (2, err, "mach_port_insert_right"); /* Now map the disk image. */ *image = 0; diff --git a/libirqhelp/irqhelp.c b/libirqhelp/irqhelp.c index 5a563a1..b94fc61 100644 --- a/libirqhelp/irqhelp.c +++ b/libirqhelp/irqhelp.c @@ -366,9 +366,15 @@ irqhelp_remove_interrupt_handler(struct irq *irq) irq->shutdown = true; /* Turn port into dead name */ - mach_port_insert_right (mach_task_self (), irq->port, irq->port, - MACH_MSG_TYPE_MAKE_SEND); - mach_port_mod_refs (mach_task_self (), irq->port, - MACH_PORT_RIGHT_RECEIVE, -1); + error_t err = mach_port_insert_right (mach_task_self (), irq->port, + irq->port, MACH_MSG_TYPE_MAKE_SEND); + if (err) + return err; + + err = mach_port_mod_refs (mach_task_self (), irq->port, + MACH_PORT_RIGHT_RECEIVE, -1); + if (err) + return err; + return 0; } diff --git a/lwip/port/netif/hurdethif.c b/lwip/port/netif/hurdethif.c index 0370281..7072026 100644 --- a/lwip/port/netif/hurdethif.c +++ b/lwip/port/netif/hurdethif.c @@ -155,85 +155,74 @@ hurdethif_device_set_flags (struct netif *netif, uint16_t flags) static error_t hurdethif_device_open (struct netif *netif) { - error_t err = ERR_OK; - device_t master_device; hurdethif *ethif = netif_get_state (netif); - if (ethif->ether_port != MACH_PORT_NULL) { error (0, 0, "Already opened: %s", ethif->devname); return -1; } - err = ports_create_port (etherread_class, etherport_bucket, - sizeof (struct port_info), ðif->readpt); + error_t err = ports_create_port (etherread_class, etherport_bucket, + sizeof (struct port_info), ðif->readpt); if (err) { error (0, err, "ports_create_port on %s", ethif->devname); + return err; } - else + + ethif->readptname = ports_get_right (ethif->readpt); + err = mach_port_insert_right (mach_task_self (), ethif->readptname, + ethif->readptname, + MACH_MSG_TYPE_MAKE_SEND); + if (err) { - ethif->readptname = ports_get_right (ethif->readpt); - mach_port_insert_right (mach_task_self (), ethif->readptname, - ethif->readptname, MACH_MSG_TYPE_MAKE_SEND); + error (0, err, "mach_port_insert_right"); + return err; + } - mach_port_set_qlimit (mach_task_self (), ethif->readptname, - MACH_PORT_QLIMIT_MAX); + mach_port_set_qlimit (mach_task_self (), ethif->readptname, + MACH_PORT_QLIMIT_MAX); - master_device = file_name_lookup (ethif->devname, O_RDWR, 0); - if (master_device != MACH_PORT_NULL) - { - /* The device name here is the path of a device file. */ - err = device_open (master_device, D_WRITE | D_READ, - "eth", ðif->ether_port); - mach_port_deallocate (mach_task_self (), master_device); - if (err) - error (0, err, "device_open on %s", ethif->devname); - else - { - err = device_set_filter (ethif->ether_port, ethif->readptname, - MACH_MSG_TYPE_MAKE_SEND, 0, - (filter_array_t) bpf_ether_filter, - bpf_ether_filter_len); - if (err) - error (0, err, "device_set_filter on %s", ethif->devname); - } - } - else + char *device_name; + filter_array_t filter; + unsigned filter_len; + device_t master_device = file_name_lookup (ethif->devname, O_RDWR, 0); + if (master_device == MACH_PORT_NULL) + { + int file_errno = errno; + err = get_privileged_ports (0, &master_device); + if (err) { - /* No, perhaps a Mach device? */ - int file_errno = errno; - err = get_privileged_ports (0, &master_device); - if (err) - { - error (0, file_errno, "file_name_lookup %s", ethif->devname); - error (0, err, "and cannot get device master port"); - } - else - { - err = device_open (master_device, D_WRITE | D_READ, - ethif->devname, ðif->ether_port); - mach_port_deallocate (mach_task_self (), master_device); - if (err) - { - error (0, file_errno, "file_name_lookup %s", - ethif->devname); - error (0, err, "device_open(%s)", ethif->devname); - } - else - { - err = - device_set_filter (ethif->ether_port, ethif->readptname, - MACH_MSG_TYPE_MAKE_SEND, 0, - (filter_array_t) ether_filter, - ether_filter_len); - if (err) - error (0, err, "device_set_filter on %s", ethif->devname); - } - } + error (0, file_errno, "file_name_lookup %s", ethif->devname); + error (0, err, "and cannot get device master port"); + return err; } + + device_name = ethif->devname; + filter = ether_filter; + filter_len = ether_filter_len; + } + else + { + device_name = "eth"; + filter = bpf_ether_filter; + filter_len = bpf_ether_filter_len; } + err = device_open (master_device, D_WRITE | D_READ, + device_name, ðif->ether_port); + mach_port_deallocate (mach_task_self (), master_device); + if (err) + { + error (0, err, "device_open(%s)", device_name); + return err; + } + + err = device_set_filter (ethif->ether_port, ethif->readptname, + MACH_MSG_TYPE_MAKE_SEND, 0, filter, filter_len); + if (err) + error (0, err, "device_set_filter on %s", device_name); + return err; } diff --git a/pfinet/ethernet.c b/pfinet/ethernet.c index bba8ff6..132513d 100644 --- a/pfinet/ethernet.c +++ b/pfinet/ethernet.c @@ -207,8 +207,10 @@ ethernet_open (struct device *dev) sizeof (struct port_info), &edev->readpt); assert_perror_backtrace (err); edev->readptname = ports_get_right (edev->readpt); - mach_port_insert_right (mach_task_self (), edev->readptname, edev->readptname, - MACH_MSG_TYPE_MAKE_SEND); + err = mach_port_insert_right (mach_task_self (), edev->readptname, + edev->readptname, MACH_MSG_TYPE_MAKE_SEND); + if (err) + error (2, err, "mach_port_insert_right"); mach_port_set_qlimit (mach_task_self (), edev->readptname, MACH_PORT_QLIMIT_MAX); diff --git a/storeio/pager.c b/storeio/pager.c index 10bdd1c..7a26efa 100644 --- a/storeio/pager.c +++ b/storeio/pager.c @@ -256,9 +256,15 @@ dev_get_memory_object (struct dev *dev, vm_prot_t prot, memory_object_t *memobj) return dev_get_memory_object (dev, prot, memobj); } else - err = - mach_port_insert_right (mach_task_self (), - *memobj, *memobj, MACH_MSG_TYPE_MAKE_SEND); + { + err = mach_port_insert_right (mach_task_self (), *memobj, *memobj, + MACH_MSG_TYPE_MAKE_SEND); + if (err) + { + mach_port_deallocate (mach_task_self (), *memobj); + *memobj = MACH_PORT_NULL; + } + } if (created) ports_port_deref (dev->pager); diff --git a/term/users.c b/term/users.c index 445196a..4ff334d 100644 --- a/term/users.c +++ b/term/users.c @@ -101,9 +101,11 @@ init_users (void) error (1, err, "mach_port_allocate"); /* Add a send right, since hurd_sig_post needs one. */ - mach_port_insert_right (mach_task_self (), - async_icky_id, async_icky_id, - MACH_MSG_TYPE_MAKE_SEND); + err = mach_port_insert_right (mach_task_self (), + async_icky_id, async_icky_id, + MACH_MSG_TYPE_MAKE_SEND); + if (err) + error (1, err, "mach_port_insert_right"); err = mach_port_allocate (mach_task_self (), MACH_PORT_RIGHT_RECEIVE, &async_id); @@ -111,8 +113,10 @@ init_users (void) error (1, err, "mach_port_allocate"); /* Add a send right, since hurd_sig_post needs one. */ - mach_port_insert_right (mach_task_self (), - async_id, async_id, MACH_MSG_TYPE_MAKE_SEND); + err = mach_port_insert_right (mach_task_self (), async_id, async_id, + MACH_MSG_TYPE_MAKE_SEND); + if (err) + error (1, err, "mach_port_insert_right"); } diff --git a/trans/streamio.c b/trans/streamio.c index 694da06..57bbf04 100644 --- a/trans/streamio.c +++ b/trans/streamio.c @@ -834,14 +834,13 @@ dev_open (const char *name, dev_mode_t mode) err = ports_create_port (phys_reply_class, streamdev_bucket, sizeof (struct port_info), &phys_reply_pi); if (err) - { - mach_port_deallocate (mach_task_self (), device_master); - return err; - } + goto device_master_deallocate; phys_reply = ports_get_right (phys_reply_pi); - mach_port_insert_right (mach_task_self (), phys_reply, phys_reply, - MACH_MSG_TYPE_MAKE_SEND); + err = mach_port_insert_right (mach_task_self (), phys_reply, phys_reply, + MACH_MSG_TYPE_MAKE_SEND); + if (err) + goto phys_reply_deallocate; if (output_buffer) { @@ -849,40 +848,41 @@ dev_open (const char *name, dev_mode_t mode) sizeof (struct port_info), &phys_reply_writes_pi); if (err) - { - mach_port_deallocate (mach_task_self (), phys_reply); - phys_reply = MACH_PORT_NULL; - ports_port_deref (phys_reply_pi); - phys_reply_pi = 0; - mach_port_deallocate (mach_task_self (), device_master); - return err; - } + goto phys_reply_deallocate; phys_reply_writes = ports_get_right (phys_reply_writes_pi); - mach_port_insert_right (mach_task_self (), phys_reply_writes, - phys_reply_writes, MACH_MSG_TYPE_MAKE_SEND); + err = mach_port_insert_right (mach_task_self (), phys_reply_writes, + phys_reply_writes, + MACH_MSG_TYPE_MAKE_SEND); + if (err) + goto phys_reply_writes_deallocate; } err = device_open_request (device_master, phys_reply, mode, (char *) name); if (err) - { - mach_port_deallocate (mach_task_self (), phys_reply); - phys_reply = MACH_PORT_NULL; - ports_port_deref (phys_reply_pi); - phys_reply_pi = 0; - if (output_buffer) - { - mach_port_deallocate (mach_task_self (), phys_reply_writes); - phys_reply_writes = MACH_PORT_NULL; - ports_port_deref (phys_reply_writes_pi); - phys_reply_writes_pi = 0; - } - mach_port_deallocate (mach_task_self (), device_master); - return err; - } + goto phys_reply_writes_deallocate; open_pending = 1; return 0; + + phys_reply_writes_deallocate: + if (output_buffer) + { + mach_port_deallocate (mach_task_self (), phys_reply_writes); + phys_reply_writes = MACH_PORT_NULL; + ports_port_deref (phys_reply_writes_pi); + phys_reply_writes_pi = 0; + } + + phys_reply_deallocate: + mach_port_deallocate (mach_task_self (), phys_reply); + phys_reply = MACH_PORT_NULL; + ports_port_deref (phys_reply_pi); + phys_reply_pi = 0; + + device_master_deallocate: + mach_port_deallocate (mach_task_self (), device_master); + return err; } kern_return_t diff --git a/trans/symlink.c b/trans/symlink.c index 05612db..536cafd 100644 --- a/trans/symlink.c +++ b/trans/symlink.c @@ -90,8 +90,11 @@ main (int argc, char **argv) if (err) error (1, err, "mach_port_allocate"); - mach_port_insert_right (mach_task_self (), control, control, - MACH_MSG_TYPE_MAKE_SEND); + err = mach_port_insert_right (mach_task_self (), control, control, + MACH_MSG_TYPE_MAKE_SEND); + if (err) + error (1, err, "mach_port_insert_right"); + err = fsys_startup (bootstrap, 0, control, MACH_MSG_TYPE_COPY_SEND, &realnode); mach_port_deallocate (mach_task_self (), control); -- 2.43.0
