Skip to content

Avoid std::terminate in FsUserContext destructor (Plan9)#40417

Open
benhillis wants to merge 2 commits intomasterfrom
copilot/fix-plan9-destructor-exception
Open

Avoid std::terminate in FsUserContext destructor (Plan9)#40417
benhillis wants to merge 2 commits intomasterfrom
copilot/fix-plan9-destructor-exception

Conversation

@benhillis
Copy link
Copy Markdown
Member

Avoid potential std::terminate from throwing in FsUserContext destructor.

Problem

FsUserContext::~FsUserContext() in src/linux/plan9/p9util.cpp used THROW_LAST_ERROR_IF() to validate that sys_setresuid, sys_setresgid, and sys_setgroups succeeded when restoring root. If this destructor runs during stack unwinding from another in-flight exception, the second exception triggers std::terminate() and aborts the process.

Fix

Replace THROW_LAST_ERROR_IF with LOG_LAST_ERROR_IF for the three syscalls. Standard pattern for destructors.

Notes

  • The constructor at lines 228, 237-238 still uses THROW_LAST_ERROR_IF correctly - failures during setup should propagate.
  • These restore-to-root syscalls running in a privileged daemon thread virtually never fail.
  • A silent restore failure would leave the thread with the wrong uid/gid, but throwing was strictly worse: process termination is non-recoverable and destroys any in-flight exception context.

FsUserContext::~FsUserContext() used THROW_LAST_ERROR_IF() which throws
exceptions. If this destructor runs during stack unwinding from another
exception, std::terminate is called immediately.

Replace with LOG_LAST_ERROR_IF() to log failures without throwing.
These syscalls (setresuid/setresgid/setgroups to restore root) should
virtually never fail, but if they do, logging is the appropriate
response in a destructor.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@benhillis benhillis requested a review from a team as a code owner May 5, 2026 00:49
Copilot AI review requested due to automatic review settings May 5, 2026 00:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the Linux Plan9 helper RAII cleanup path by preventing FsUserContext teardown from throwing during exception unwinding. It fits the codebase by making temporary UID/GID impersonation cleanup follow the usual destructor-safe logging pattern instead of terminating the process.

Changes:

  • Replaced throwing restore syscalls in FsUserContext::~FsUserContext() with LOG_LAST_ERROR_IF.
  • Preserved constructor behavior so setup failures still propagate immediately.
  • Limited the change to the Plan9 Linux utility cleanup path.

OneBlue
OneBlue previously approved these changes May 5, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants