Re,
Alperen Erkan, le dim. 20 sept. 2026 11:56:27 +0300, a ecrit:
> Key Changes:
Again, it contains several unrelated changes, better commit them
separately to be able to git bisect
> Selection and Readable Routines: The S_io_readable function has been made to
> safely check ioctl return values.
That belongs to the second patch.
> The `S_io_select` and `S_io_select_timeout` functions have been
> redirected via the new `queue_select` mechanism,
Why doing this? The existing code is very simple to maintain.
> which includes appropriate time validation (EINVAL check).
That can be added easily.
> `S_io_stat` and Character Devices: The `S_io_stat` function has been updated
> to
> explicitly report a character device (`S_IFCHR | 0666`).
That should be separate.
> Authentication Clean-up: `S_io_reauthenticate` now checks for valid objects
> and
> the authentication server.
That poses question:
> @@ -2097,11 +2104,18 @@ S_io_reauthenticate (mach_port_t object,
> mach_msg_type_number_t gulen = 0, aulen = 0, gglen = 0, aglen = 0;
> error_t err;
>
> - /* XXX: This cannot possibly work, authserver is 0. */
> + if (object != pseudo_console)
> + return EOPNOTSUPP;
> +
> + /* Without an auth server there is nobody to reauthenticate
> + against. */
> + if (authserver == MACH_PORT_NULL)
> + return EOPNOTSUPP;
The existing comment says that authserver is zero, raising the concern
that all this cannot actually work. Here you are dropping that comment.
Is that because nowadays authserver is not nul because of other fixes?
Otherwise we have to keep the comment. Also, is your check perhaps
breaking something because not checking for authserver being nul was
actually needed to get things to work (even if improperly)?
> Memory deallocation errors in `S_io_reauthenticate`,
> where incorrect array sizes and pointers were passed to the `mig_deallocate`
> function, have been fixed.
They were not incorrect since these are the same types in the end. But
better clean that up, yes. Again, better separate this from patches
which actually change the behavior.
> Security and Port Checks: A pseudo_privileged_host_port validation has been
> added to the S_host_reboot function. printf format strings for Mach ports have
> been corrected (%lu).
Ok.
> Task Notification and Listing: Error handling in the `S_mach_notify_new_task`
> function has been updated to correctly deallocate memory for tasks and parent
> ports in the event of an error.
? No. In case of errors, the mig caller will handle deallocations.
Also,
> @@ -2505,15 +2505,15 @@ S_mach_notify_new_task (mach_port_t notify,
> err = hurd_ihash_add (&task_ihash,
> (hurd_ihash_key_t) task,
> (hurd_ihash_value_t)(uintptr_t) task);
> if (err)
> - {
> - mach_port_deallocate (mach_task_self (), task);
> - goto fail;
> - }
> + goto fail;
That is wrong: the deallocation here is matched with the
mach_port_mod_refs just above. This is very different from deallocating
the ports passed to the RPC (which, again, the mig caller will handle).
> The `S_processor_set_tasks` function has been
> updated with overflow checks, and it has been ensured that `pseudo_kernel` is
> listed correctly even if it is not present in the hash table.
That also deserves its own patch.
Samuel
