Page MenuHomeFreeBSD

ithread: Improve synchronization in ithread_destroy()
ClosedPublic

Authored by markj on Jun 5 2024, 1:22 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Sep 25, 1:09 PM
Unknown Object (File)
Tue, Sep 24, 3:24 PM
Unknown Object (File)
Tue, Sep 24, 3:24 PM
Unknown Object (File)
Tue, Sep 24, 3:24 PM
Unknown Object (File)
Sun, Sep 22, 7:26 PM
Unknown Object (File)
Wed, Sep 18, 9:20 AM
Unknown Object (File)
Wed, Sep 18, 9:19 AM
Unknown Object (File)
Wed, Sep 18, 9:19 AM
Subscribers

Details

Summary

Previously, to destroy an ithread we would set IT_DEAD in its flags, and
then wake it up if it wasn't already running. After doing this,
intr_event_destroy() would free the intr_event structure. However, it
did not wait for the ithread to exit, so it was possible for the ithread
to access the intr_event after it was freed.

This use-after-free happens readily when running the pf tests in
parallel, since they frequently create and destroy VNET jails, and pf
registers several VNET-local swi handlers.

Fix the race by modifying ithread_destroy() to wait until the ithread
has signaled that it is about to exit by setting ie->ie_thread = NULL.
Existing callers of intr_event_destroy() are allowed to sleep.

Reported by: KASAN

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 58061
Build 54949: arc lint + arc unit

Event Timeline

markj requested review of this revision.Jun 5 2024, 1:22 AM

The bug is causing panics while running our test suite for a long time, most recently: https://ci.freebsd.org/job/FreeBSD-main-amd64-test/25367/console

I would like to commit it soon if there are no objections.

kib added inline comments.
sys/kern/kern_intr.c
584

Should we assert ie_lock there?

1311
This revision is now accepted and ready to land.Jul 30 2024, 1:06 AM
markj added inline comments.
sys/kern/kern_intr.c
584

Yes, that makes sense, I did that locally.

This revision was automatically updated to reflect the committed changes.
markj marked an inline comment as done.