Closed Bug 1662560 Opened 3 months ago Closed 2 months ago

VirtualProtectEx for a sandboxed process fails with ERROR_INVALID_ADDRESS

Categories

(Firefox :: Launcher Process, defect, P3)

Unspecified
Windows
defect

Tracking

()

RESOLVED FIXED
82 Branch
Tracking Status
firefox82 --- fixed

People

(Reporter: toshi, Assigned: toshi)

References

(Blocks 2 open bugs)

Details

Attachments

(4 files)

This is one of the top failures to launch a sandboxed process.

This can happen even when the launcher process is disabled, ending up with the no-content-process situation. So the severity may be a bit higher.

Our data indicates when the browser process populates a newly-created child process,
VirtualProtectEx may fail with ERROR_INVALID_ADDRESS for some unknown reason.

One possible cause is the parameter aRemoteExeImage of RestoreImportDirectory
was wrong i.e. pointing to an invalid address. We simply pass the local process's
imagebase as aRemoteExeImage based on the assumption that the same executable is
mapped onto the same address in a different process, but it may not be guaranteed.

To deal with that potential case, we could retrieve a correct imagebase from the handle
of a remote process as we do for the plugin process. Since we're not so sure about
the root cause or the effectiveness of this fix, we run it only when the first
attempt to VirtualProtectEx failed in Nightly. Once it's confirmed, we promote this
to a permanent fix.

Assignee: nobody → tkikuchi
Status: NEW → ASSIGNED

I wonder if at least some of these failures are another symptom of the situation that's being encountered in (for example) bug 1120863, where we have one Firefox instance that starts up and applies an update while another instance was still running. If the preexisting instance tries to start a new content process after that happens, it would be using the updated image, so the old image which is still running in the parent wouldn't be mapped there at all. This also means the new process is not actually going to be usable later on, which is what the other bugs about that problem are seeing the effects of.

We do have a few efforts under way (in various stages of completion) which attempt to mitigate that problem, but I don't think we'll be able to eradicate it completely, so defensive efforts like this patch will still be warranted.

Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ea452bb92e6a
Retrieve the imagebase of the child process's executable from a process handle.  r=mhowell
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch
Status: RESOLVED → REOPENED
Keywords: leave-open
Resolution: FIXED → ---

Our latest data shows no failures at VirtualProtectEx with this fix. This means relying on the assumption that the same executable will be mapped onto the same address is risky. A symptom could be worse if we modified data at an address in a child process that is valid but not the imagebase.

The earlier fix ea452bb92e6a proved the executable's imagebase in a child
process is not always the same as the local imagebase. This patch applies
the new approach to retieve the imagebase from a handle to all channels.

Interestingly, we observed the launcher failures at VirtualProtectEx only
when launching a sandboxed process, not when launching the browser process.
In the long term, we may need to take care of all WriteProcessMemory calls
for a child process for greater safety, but given that observation, this
patch only updates RestoreImportDirectory for now.

Pushed by cbrindusan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/93bb035ef2c9
Always retrieve the imagebase of the child process's executable from a process handle. r=mhowell
Status: REOPENED → RESOLVED
Closed: 3 months ago2 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.