Bug 1850969 Comment 4 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Greg Stoll from comment #3)
> The minidump indicates that `sSectionHandle` is null, although I'm not sure how much we can trust that.

We can't trust static variables values' at all, I'm afraid. (I assume they're not actually in the minidump, and that they only appear to have a value because the minidump specifies where the `.data` or `.bss` segments get loaded. :gsvelto would know.)

> I took a look at [this crash report](https://crash-stats.mozilla.org/report/index/69dfb7e5-7188-4d82-a08c-f88f40230907) - it has WRusr.dll loaded. (note that earlier versions of WRusr.dll are on our blocklist, but this one is late enough that it shouldn't be getting blocked) The value in `rax` is 5, which suggests that `NtMapViewOfSection` might be returning `ERROR_ACCESS_DENIED`. (although [`ZwMapViewOfSection`](https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-zwmapviewofsection) is supposed to return an `NTSTATUS`, not sure if this is true for the "Nt" version as well?)

It is, but [we immediately convert it](https://searchfox.org/mozilla-central/rev/146638366864f73ee8a697ea76480eb02c00eb3c/mozglue/misc/NativeNt.h#1691-1693); interpreting it as `ERROR_ACCESS_DENIED` seems to be the right call, assuming it went through that code path at all...

... which it almost certainly must have done. If it didn't, then `rax` would be a value left over from the `__security_cookie` computation -- which could be anything, but is vastly more likely to have junk in the high bits than to be `0x0000000000000005`.

On the other hand, if it _did_ go through that code path, that means that `NtMapViewOfSection` is [storing a value into `mView`](https://searchfox.org/mozilla-central/rev/146638366864f73ee8a697ea76480eb02c00eb3c/mozglue/misc/NativeNt.h#1688-1690) (here, r14 = `0000025D24870000`), despite also returning a failure code. Which would be very odd, but I can't trivially rule it out...

Ah.

`NtMapViewOfSection` itself probably never does that, but we're hooking it -- and bug 1733532 / [patch D182982](https://phabricator.services.mozilla.com/D182982), which was also added in 117 and uplifted to 115.0.2esr, added [a return path](https://searchfox.org/mozilla-central/source/browser/app/winlauncher/freestanding/DllBlocklist.cpp#574-577) which returns `ERROR_ACCESS_DENIED` without zeroing `*aBaseAddress`.

Marking as regression accordingly.
(In reply to Greg Stoll from comment #3)
> The minidump indicates that `sSectionHandle` is null, although I'm not sure how much we can trust that.

We can't trust static variables values' at all, I'm afraid. (I assume they're not actually in the minidump, and that they only appear to have a value because the minidump specifies where the `.data` or `.bss` segments get loaded. :gsvelto would know.)

> I took a look at [this crash report](https://crash-stats.mozilla.org/report/index/69dfb7e5-7188-4d82-a08c-f88f40230907) - it has WRusr.dll loaded. (note that earlier versions of WRusr.dll are on our blocklist, but this one is late enough that it shouldn't be getting blocked) The value in `rax` is 5, which suggests that `NtMapViewOfSection` might be returning `ERROR_ACCESS_DENIED`. (although [`ZwMapViewOfSection`](https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-zwmapviewofsection) is supposed to return an `NTSTATUS`, not sure if this is true for the "Nt" version as well?)

It is, but [we immediately convert it](https://searchfox.org/mozilla-central/rev/146638366864f73ee8a697ea76480eb02c00eb3c/mozglue/misc/NativeNt.h#1691-1693); interpreting it as `ERROR_ACCESS_DENIED` seems to be the right call, assuming it went through that code path at all...

... which it almost certainly must have done. If it didn't, then `rax` would be a value left over from the `__security_cookie` computation -- which could be anything, but is vastly more likely to have junk in the high bits than to be `0x0000000000000005`.

On the other hand, if it _did_ go through that code path, that means that `NtMapViewOfSection` is [storing a value into `mView`](https://searchfox.org/mozilla-central/rev/146638366864f73ee8a697ea76480eb02c00eb3c/mozglue/misc/NativeNt.h#1688-1690) (here, r14 = `0000025D24870000`), despite also returning a failure code. Which would be very odd, but I can't trivially rule it out...

Ah.

`NtMapViewOfSection` itself probably never does that, but we're hooking it -- and bug 1733532 / [patch D182982](https://phabricator.services.mozilla.com/D182982), which was also added in 117 and uplifted to 115.0.2esr, added [a return path](https://searchfox.org/mozilla-central/source/browser/app/winlauncher/freestanding/DllBlocklist.cpp#574-577) to our hook which returns `ERROR_ACCESS_DENIED` without zeroing `*aBaseAddress`.

Marking as regression accordingly.

Back to Bug 1850969 Comment 4