Hi Samuel,

Thanks for the review.
Unfortunately, i only have the victim thread, not the destruction thread.
Here is the backtrace of the crash:

Thread 26 (Thread 759.26):
#7  0x08048650 in __assert_fail_base_backtrace (fmt=...,
assertion=0x81e3fa0 "! (r.hard == 1 && r.weak == 0) || !\"refcount detected
use-after-free!\""...) at ../../libshouldbeinlibc/assert-backtrace.c:59
#9  0x0808c03e in refcounts_ref (ref=<optimized out>, result=0x0) at
../../libports/../libshouldbeinlibc/refcount.h:170
#10 _ports_bucket_class_iterate (ht=<optimized out>, class=0x0,
fun=0x1ea68f4c) at ../../libports/bucket-iterate.c:61
#11 0x0808c0a6 in ports_bucket_iterate (bucket=0x20009810, fun=0x1ea68f4c)
at ../../libports/bucket-iterate.c:93
#12 0x08056261 in diskfs_sync_everything (wait=0) at
../../ext2fs/pager.c:1598
#13 0x08061bc2 in periodic_sync (arg=0x1e) at
../../libdiskfs/sync-interval.c:126

Since this triggers during diskfs_sync_everything while heavily stressing
the system with rm -rf and tar, the dying port is likely a file pager port
or node port being aggressively torn down by the VM (e.g., falling through
ports_port_deref).

The mechanics of the race seem to be:
    - The destruction thread atomically drops the port's hard refcount to 0.
    - It blocks waiting to acquire the _ports_htable_lock write-lock to
actually remove the port from the hash table.
    - The periodic_sync thread, holding the read-lock, iterates over the
bucket, finds the port still in the ihash, and calls refcounts_ref(),
hitting the use-after-free assertion.

I want to write a patch to make the destruction path more cautious but
because dropping the refcount is atomic, and removing it from the ihash
requires the write-lock, there is an inherent race window unless we acquire
the write-lock before dropping the final reference (which would cause
massive lock contention).

Do you have a preferred design in mind for how libports should safely hide
these dying ports from the read-locked iterators without killing
performance?

Thanks,
Milos

On Mon, Sep 7, 2026 at 3:40 PM Samuel Thibault <[email protected]>
wrote:

> Hello,
>
> Milos Nikic, le dim. 06 sept. 2026 00:27:32 -0700, a ecrit:
> > When heavily stressed (e.g., rapid file deletions during a tar
> extraction),
> > a race condition occurs in _ports_bucket_class_iterate.
> >
> > A destruction thread may drop a port's final reference (refcount hits 0)
> > but get blocked waiting for the _ports_htable_lock write-lock to remove
> > it from the ihash table.
>
> Could you tell us which destruction path that is?
>
> E.g. _ports_complete_deallocate takes care of checking the references
> against getting reacquired.
>
> > If the periodic sync thread simultaneously holds
> > the read-lock and iterates over the bucket, it encounters this "dead"
> port.
> > Attempting to call refcounts_ref() on it triggers a use-after-free
> assertion
> > panic in the kernel.
> >
> > Since the read-lock guarantees the memory is still safe, we can simply
> > check if the hard refcount is 0 and skip the port, allowing the teardown
> > thread to finish its job once the lock is released.
>
> This approach is a bit fishy as in it requires all iterators over the
> bucket to check for dying ports. There is this one, but also
> ports_inhibit_bucket_rpcs, and whatnot that could be added later. If we
> can rather make the destruction paths (which is normally not diverse) be
> more cautious, we'll avoid the issue for all iterators cases.
>
> Thanks,
> Samuel
>
> > ---
> >  libports/bucket-iterate.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/libports/bucket-iterate.c b/libports/bucket-iterate.c
> > index 9103c2977..7ac18a91d 100644
> > --- a/libports/bucket-iterate.c
> > +++ b/libports/bucket-iterate.c
> > @@ -58,6 +58,13 @@ _ports_bucket_class_iterate (struct hurd_ihash *ht,
> >
> >        if (class == 0 || pi->class == class)
> >       {
> > +         /* Check if the port is already dying.
> > +            Because we hold the read lock, the memory is safe,
> > +            but if the hard refcount is 0, we must skip it to
> > +            prevent a use-after-free panic! */
> > +       if (refcounts_hard_references (&pi->refcounts) == 0)
> > +            continue;
> > +
> >         refcounts_ref (&pi->refcounts, NULL);
> >         p[n] = pi;
> >         n++;
> > --
> > 2.55.0
>

Reply via email to