1410241 - SIGSYS handler can access destroyed objects during shutdown

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, enhancement)

Description

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

We're storing the client objects for the sandbox file broker and rejected syscall reporter in static variables, which means they'll be destroyed on shutdown when static destructors are calls.

However, they could be used by a SIGSYS handler after that, or concurrently in a SIGSYS handler on another thread (possibly even a call which started before shutdown).

I remembered not to do this when I was writing the SandboxOpenedFiles code

I'm reasonably sure that this invokes undefined behavior regardless of what the destructors actually do.  If the compiler doesn't do anything clever, then SandboxReporterClient is unaffected, but SandboxBrokerClient will do a file descriptor use-after-close, sending the broker request on some other fd (possibly its own response socket).  And, of course, it's not a good idea to depend on the compiler not being clever.

Comment 1

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

(In reply to Jed Davis [:jld] (⏰UTC-6) from comment #0)
> I remembered not to do this when I was writing the SandboxOpenedFiles code

I think the end of this sentence was going to be something like “but not for SandboxBroker or SandboxReporter”.

Anyway, I'm taking this bug because the patch is simple and I wrote it.

Comment 2

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Attachment: Bug 1410241 - Don't call destructors on objects we use in the SIGSYS handler.

Review commit: https://reviewboard.mozilla.org/r/193182/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/193182/

Comment 3

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8922165 [details]
Bug 1410241 - Don't call destructors on objects we use in the SIGSYS handler.

https://reviewboard.mozilla.org/r/193182/#review198686

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7e53467676ca
Don't call destructors on objects we use in the SIGSYS handler. r=gcp

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/7e53467676ca