1163452 - change assertion into check for MozillaRegisterDebugHandle

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: change assert into check

[not sure if IO poisoning goes in this component or not]

When a Firefox build is started under a msys2 shell, this assert fires.  I'm guessing under the hood msys2 is merging stdout and stderr.  This fixes it, but I don't know what the effects of it migt be...

Comment 1

[Mike Hommey [:glandium]]

Comment on attachment 8603903 [details] [diff] [review]
change assert into check

Review of attachment 8603903 [details] [diff] [review]:
-----------------------------------------------------------------

::: xpcom/build/PoisonIOInterposerBase.cpp
@@ +227,4 @@
>  {
>    DebugFilesAutoLock lockedScope;
>    FdList& DebugFileIDs = getDebugFileIDs();
> +  if (!DebugFileIDs.Contains(aHandle)) {

Looking at the code, my guess as to why this is happening is because on windows, both _get_osfhandle(1) and _get_osfhandle(2) return the same handle with msys2. You should be able to double check this with the process explorer.

Anyways, this test has been a MOZ_ASSERT since the beginning of time of poisoning, which predates me ever touching it. I'd say it's a good thing it is a MOZ_ASSERT and we should try to keep it that way as much as possible.

So I'd rather go with something like only registering file descriptor 2 in InitPoisonIOInterposer in PoisonIOInterposerWin.cpp if e.g. GetStdHandle(STD_OUTPUT_HANDLE) != GetStdHandle(STD_ERROR_HANDLE), or something along these lines.

Comment 2

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1163452 - Only register stderr if it differs from stdout. r?glandium

Under certain Windows shells, the handle for stderr may be the same as
the handle for stdout. Avoid crashing in this case.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b60e9cff74df
Only register stderr if it differs from stdout. r=glandium

Comment 4

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/b60e9cff74df