Page MenuHomeFreeBSD

fusefs: Fix a panic when unmounting before init
ClosedPublic

Authored by asomers on Jun 11 2025, 8:33 PM.
Tags
None
Referenced Files
F132292475: D50799.diff
Wed, Oct 15, 2:33 PM
Unknown Object (File)
Sun, Oct 12, 11:46 PM
Unknown Object (File)
Sun, Oct 12, 11:46 PM
Unknown Object (File)
Sun, Oct 12, 11:46 PM
Unknown Object (File)
Sun, Oct 12, 11:46 PM
Unknown Object (File)
Sun, Oct 12, 12:17 PM
Unknown Object (File)
Sun, Oct 12, 2:48 AM
Unknown Object (File)
Mon, Sep 15, 5:46 PM
Subscribers

Details

Summary

fusefs would page fault due to the following sequence of events:

  • The server did not respond to FUSE_INIT in timely fashion.
  • Some other process tried to do unmount.
  • That other process got killed by a signal.
  • The server finally responded to FUSE_INIT.

PR: 287438
MFC after: 2 weeks
Sponsored by: ConnectWise

Test Plan

Test case added

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

I mostly have no idea how this code works, but I tried to provide some review.

sys/fs/fuse/fuse_internal.c
982

I grepped for fdata_get_dead() and stumbled upon its usage in fuse_device_poll() which confused me. I was researching this topic just recently and learned that on FreeBSD we return revents = POLLHUP to report the EOF state. But fuse_device_poll() does not return POLLHUP nor POLLERR in the fdata_get_dead() case.

tests/sys/fs/fusefs/pre-init.cc
89

Why capture anything there?

106

If this nap is used to wait for the thread to finish, maybe it'd be better to use pthread_join()?

154

How about calling std::unreachable() here to have some visibility in case we actually reach this line?

157

Just like in the case of threads, wouldn't waitpid() be better here?

sys/fs/fuse/fuse_internal.c
982

Good find. I'll take a look at that next.

tests/sys/fs/fusefs/pre-init.cc
61

This part is actually unnecessary leftovers from an earlier iteration. I'll delete it.

89

Because I copy/pasted from somewhere else. I'll delete it.

106

It isn't. In fact, the thread can't finish before sem_post. Instead, nap() is just used for the thread to start and get blocked within sys_unmount().

154

That's a good idea. However, std::unreachable requires C++23, and our global C++ standard is only gnu++17. Plus, I just realized that it isn't technically unreachable; if the parent gets killed after forking the child could still technically get here. I'll change the comment.

157

As in the other case, the child process will never finish until we kill it. So we can't waitpid() before kill().

But the nap() below the kill is different. I originally used it because at first I thought that triggering the panic would require fuse_internal_init_callback to be called before fuse_vfsop_unmount finished. But then I discovered that it doesn't. So I can replace that.

  • Respond to gleb's comments

LGTM, but my review doesn't worth much.

This revision is now accepted and ready to land.Jun 12 2025, 4:11 PM

LGTM, but my review doesn't worth much.

It totally was. You found multiple good things.