1322610 - Crash in mozilla::net::CrashWithReason - GetValidatedOriginAttributes | SerializedLoadContext from child is null

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Valentin Gosu [:valentin] (he/him)]

This bug was filed from the Socorro interface and is 
report bp-732ced99-848f-4ba3-963f-219152161207.
=============================================================

The container issues from bug 1317611 seem to have been fully fixed by bug 1315905, but we still get a few of them with a null load context.
It's possible all of the crashes come from the same user. I suspect that this is because of an addon:
(100.0% in signature vs 00.12% overall) Addon "Diccionario español México" = true
(100.0% in signature vs 01.29% overall) Addon "Xmarks Sync" = true
(100.0% in signature vs 02.41% overall) Addon "SQLite Manager" = true
(83.33% in signature vs 00.04% overall) Addon "easy Xdebug (with moveable icon)" = true
(83.33% in signature vs 00.35% overall) Addon "The easiest Xdebug" = true
(83.33% in signature vs 00.57% overall) Addon "FavIcon Reloader up to FF 48 only" = true
(83.33% in signature vs 01.44% overall) Addon "µBlock" = true

Comment 1

[Ting-Yu Chou [:ting] (away)]

Top #2 of Nightly 20170611100318 on Linux, there are 21 reports from 6 installations in the past week.

Comment 2

[Valentin Gosu [:valentin] (he/him)]

Most of the comments mention clicking a link to open a PDF

Comment 3

[Valentin Gosu [:valentin] (he/him)]

What we need here is to identify the reason why the SerializedLoadContext is null. I intend to land a patch that changes HttpChannelChild to (diagnostic) assert that it is not null, thus crashing only the child process, and maybe it could get us a nice stack trace to find the culprit.

Comment 4

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1322610 - MOZ_DIAGNOSTIC_ASSERT that serialized load context is not null in HttpChannelChild

As part of network security, we currently crash the main process in NeckoParent::GetValidatedOriginAttributes if the SerializedLoadContext is null. This makes it difficult to find out who created the channel that has a null SLC. Adding this assertion to the child allows us to get a nice stack trace and maybe identify the root cause.

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

Comment 5

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8877563 [details]
Bug 1322610 - MOZ_DIAGNOSTIC_ASSERT that serialized load context is not null in HttpChannelChild

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/149046/diff/1-2/

Comment 6

[Valentin Gosu [:valentin] (he/him)]

This is probably just one user that has the pref turned on for some reason, and is crashing.
The patch should show us why he is crashing (likely an addon). However, this assertion will prove useful post-57 when we'll try to make the changes Jason suggested in bug 1306801 comment 4.

Comment 7

[Darkspirit]

(In reply to Valentin Gosu [:valentin] from comment #6)
> This is probably just one user that has the pref turned on for some reason, and is crashing.
network.disable.ipc.security;false


Hello, I am maybe "the one" who gets crashes with [@ mozilla::net::NeckoParent::GetValidatedOriginAttributes ].
https://crash-stats.mozilla.com/report/index/0a1275b3-d959-40cf-9080-07caa0170615 for example

This happens when I open PDF files and pdfjs.migrationVersion=2. I can migrate that by resetting (deleting) that pref.

browser.link.open_newwindow.restriction;0
browser.photon.structure.enabled;true
browser.tabs.remote.autostart;true
browser.tabs.remote.force-enable;true
dom.disable_beforeunload;true
dom.ipc.cpows.allow-cpows-in-compat-addons;
dom.ipc.processCount;100
extensions.interposition.enabled;false <--------------------
extensions.legacy.enabled;false <--------------------
extensions.legacy.exceptions; //this manual change from me is only visual and only hides the legacy default theme
extensions.pocket.enabled;false
extensions.webextensions.themes.icons.enabled;true
extensions.webextensions.remote;false //sometimes true
gfx.webrender.enabled;true <--------------------
gfx.webrendest.enabled;true
gl.require-hardware;true
javascript.options.wasm;false
layers.acceleration.force-enabled;true
layers.gpu-process.enabled;true
layers.gpu-process.force-enabled;true
layers.gpu-process.max_restarts;50
media.eme.chromium-api.enabled;false
media.gmp-gmpopenh264.enabled;false
media.gmp-widevinecdm.enabled;false
media.gmp-widevinecdm.visible;false
media.hardware-video-decoding.force-enabled;true
network.IDN_show_punycode;true
network.auth.subresource-http-auth-allow;0
network.disable.ipc.security;false <--------------------
network.http.altsvc.oe;false
network.http.enforce-framing.http1;true
network.http.enforce-framing.soft;false
network.http.fast-fallback-to-IPv4;false
network.standard-url.enable-rust;true
network.tcp.tcp_fastopen_enable;true
plugin.allowed_types;
plugin.default.state;0 <--------------------
plugin.defaultXpi.state;2 (default) <----- can I set this to 0?
plugin.disable;true <--------------------
plugin.disable_full_page_plugin_for_types;application/pdf
plugin.state.flash;0
plugin.state.java;0
privacy.popups.disable_from_plugins;3
security.cert_pinning.enforcement_level;2
security.pki.sha1_enforcement_level;1
security.ssl.errorReporting.automatic;true
security.ssl.treat_unsafe_negotiation_as_broken;true //dreaming of bug 1351684
security.ssl3.* // disbled some ciphers, I know what I am doing here
security.tls.enable_0rtt_data;false //for now
security.tls.version.min;3
security.tls13.aes_128_gcm_sha256;false
security.xpconnect.plugin.unrestricted;false <--------------------
xpinstall.signatures.required;false <--------------------
(these are not all prefs)

I am testing webrender and report bugs, also I want to get rid of non-webextensions and NPAPI. I don't need the PPAPI for Flash (would be cool if Firefox would ship a PPAPI-Flash like Chrome in the GMP-OpenH264 way some day) from project Mortar, but I am interested in Mortars PDF viewer which I haven't seen yet. I am following some post-57 bugs like bug 1347507.

Comment 8

[Darkspirit]

(In reply to Valentin Gosu [:valentin] from comment #6)
> The patch should show us why he is crashing (likely an addon). However, this

The built-in pdfium. I do not have all addons from comment 0.
My webext-only addons:
1) IPvFoo
2) Redirector
3) uBlock Origin
4) uMatrix
No plugins (as you can see in the prefs).

how to get 1)
1. https://addons.mozilla.org/firefox/addon/chrome-store-foxified/
2. save as xpi file: https://chrome.google.com/webstore/detail/ipvfoo/ecanpcehffngcegjmadlcijfolapggal (it's like IPvFox, but a webextension) and import from file into about:addons

2) https://addons.mozilla.org/firefox/addon/redirector/

how to get 3)
I opened https://github.com/gorhill/uBlock/issues/2576 because uBlock Origin has the Legacy badge.
"The webext version of uBO is a hybrid, a webext wrapped into a thin layer legacy one, necessary for a seamless transition."
So I go to https://github.com/gorhill/uBlock/releases
Ctrl+F: webext.xpi
Download, unzip, go into the "webextension" folder, select all, zip, import into about:addons from file.

4) Same as 3, https://github.com/gorhill/uMatrix/releases

Comment 9

[Darkspirit]

network.disable.ipc.security;false (default=true)

https://gopherproxy.meulie.net/ygrex.ru/h/debian/firefox-security.html
> Interprocess security checks (disabled by default per bug 820712):
> network.disable.ipc.security=false

I just thought that "disabling" "security" is needed for some old addons or plugins, so I set it to false. "Security checks" sounded good.

(In reply to Valentin Gosu [:valentin] from comment #6)
> useful post-57 when we'll try to make the changes Jason suggested in bug 1306801 comment 4.
Then I will reset that pref for now. Thank you.

Comment 10

[Patrick McManus [:mcmanus]]

Comment on attachment 8877563 [details]
Bug 1322610 - MOZ_DIAGNOSTIC_ASSERT that serialized load context is not null in HttpChannelChild

https://reviewboard.mozilla.org/r/149046/#review154068

valentin, I'm ok with this if it will help move things forward.. but it sounds like you may have found a user to repro and don't need it?

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/016bcae14073
MOZ_DIAGNOSTIC_ASSERT that serialized load context is not null in HttpChannelChild r=mcmanus

Comment 12

[Valentin Gosu [:valentin] (he/him)]

(In reply to Patrick McManus [:mcmanus] from comment #10)
> Comment on attachment 8877563 [details]
> Bug 1322610 - MOZ_DIAGNOSTIC_ASSERT that serialized load context is not null
> in HttpChannelChild
> 
> https://reviewboard.mozilla.org/r/149046/#review154068
> 
> valentin, I'm ok with this if it will help move things forward.. but it
> sounds like you may have found a user to repro and don't need it?

Thanks for the review. This will be useful when we land 1306801, to ensure we aren't missing load context in the child.

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/016bcae14073

Comment 14

[Ryan VanderMeulen [:RyanVM]]

IIUC, this crash is Nightly-only? Also, should this bug have been closed? Looks like it was just a diagnostic patch that landed.

Comment 15

[Valentin Gosu [:valentin] (he/him)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #14)
> IIUC, this crash is Nightly-only? Also, should this bug have been closed?
> Looks like it was just a diagnostic patch that landed.

comment 7 revealed that why this crash is happening. The patch we landed will prove useful once we try to remove the pref and have the default be to always enforce having a correct serializedLoadInfo.
Since we can only get this crash by fiddling with these prefs, I think we can ignore the crash for now, and address these issues in bug 1306801 in fx57.