Closed Bug 1163452 Opened 9 years ago Closed 1 year ago

change assertion into check for MozillaRegisterDebugHandle

Categories

(Core :: Security, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
104 Branch
Tracking Status
firefox104 --- fixed

People

(Reporter: vlad, Assigned: nalexander)

References

Details

Attachments

(2 files)

[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...
Attachment #8603903 - Flags: review?(mh+mozilla)
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.
Attachment #8603903 - Flags: review?(mh+mozilla) → feedback-

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

Assignee: nobody → nalexander
Status: NEW → ASSIGNED
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b60e9cff74df
Only register stderr if it differs from stdout. r=glandium
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 104 Branch
You need to log in before you can comment on or make changes to this bug.