Re: [RFC PATCH glibc 26/34] hurd: Remove __hurd_local_reply_port

[Sergey Bugaev]

On Tue, Apr 11, 2023 at 1:07 AM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> I believe we still want to use
>
>  mach_port_t port = get_reply_port();
>
> because the caller might not know whether its port is still valid
> or not, e.g. when a signal interrupted the RPC and thus we had to
> deallocate the reply port to make sure the server doesn't get confused.
> In that case the caller will still have the old reply port name, which
> we don't want to reallocate since it might already have been reused for
> something else.

*Great* point!

(And I should have thought of that..., but hey, this is what code reviews
are for, right?)

Side note, I really really dislike this idea of some code still referencing
port names that are no longer valid / deallocated / reused by someone else.
This is really prone to use-after-frees. Typically we'd solve this by
leaving a dead-name right in place of the port, and having
mig_dealloc_reply_port () dealloc this dead name.

But in this case... we're fairly sure that the code really doesn't do
anything with the name that it has, other than immediately calling
mig_dealloc_reply_port () on it; and there'd have to be a separate code path
for deallocating the dead name since mach_port_mod_refs (recv, -1) won't do
it (mach_port_destroy would handle both, but using that is a terrible idea).

So in order not to overcomplicate this, in this particular case, it should
be fine to just deallocate the stored reply port and not what the user has,
as you're saying. But it definitely needs a comment explaining this. And
maybe an assert (port == arg || port == MACH_PORT_NULL).

Sergey