This query also shows a renderer sending an oversized IPC from what looks like (probably) a content script using a message port:

https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.magic_signature_1.name%3D%27IPC%3A%3AChannelProxy%3A%3ASendInternal%27%20AND%20product.version%20%3E%3D%20%2763.0.3219.0%27%20AND%20%20custom_data.ChromeCrashProto.ptype%3D%27renderer%27&sql_dialect=dremelsql&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D

I expect the number of crashes in that query to grow.

Actually if the numbers are high enough it might be worth blocking stable release on getting this fixed. An extension process dying is one thing, but if a content script can crash a tab that seems quite a bit more unfortunate.

How much is "too much data"?  Is this restriction new, or did extensions just start sending more?

No, it's an old restriction but it used to go undetected by crash reports,
resulting in silent extension or renderer process death. The limit is 128
MB.

FWIW on second glance it looks like *all* of the reports are in fact from message port usage, so fixing this may be as simple as imposing a max message length in the messaging JS shim.

Yeah, messaging is one of the few areas where we allow arbitrary large data.  So this isn't regressing in any way, but now we can quantify how bad it is.  Nifty.

I shudder to think about what extensions are passing 128 MB through JS, though...

Note that this means an extension can also cause the browser itself to send an oversized IPC to a renderer or extension process, resulting in us hitting the CHECK in the browser as well:

https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.magic_signature_1.name%3D%27IPC%3A%3AChannelProxy%3A%3ASendInternal%27%20AND%20custom_data.ChromeCrashProto.ptype%3D%27browser%27&sql_dialect=dremelsql&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D

Arguably we should not crash the browser in such cases, but the net result before the CHECK would still have been a renderer or extension process silently breaking. Crashing may be better than that.

Issue 766691 has been merged into this issue.

Users experienced this crash on the following builds:

Win Canary 63.0.3220.0 -  1.37 CPM, 18 reports, 17 clients (signature IPC::ChannelProxy::SendInternal)
Mac Canary 63.0.3220.0 -  3.89 CPM, 14 reports, 12 clients (signature IPC::ChannelProxy::SendInternal)

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

Issue 766032 has been merged into this issue.

Just to update the latest behavior of the crash:

Still we are seeing the crashes on latest dev:

63.0.3218.0	54.17%	130	-Dev

Link to the list of builds:
----------------------------
https://crash.corp.google.com/browse?q=product.name%3D%27Chrome_Mac%27%20%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D%27mojo%3A%3Aedk%3A%3ANodeChannel%3A%3AWriteChannelMessage%27&sql_dialect=dremelsql&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D#productversion:1000

Thanks..!

Updating the blocker and milestone label for tracking.

My understanding from rockot@ is that this is *not* a regression.  Is that accurate?

Yes, accurate. There is no net decrease in stability, only an increase in
visibility.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a31644c6bbfce45f46d92f6bb8ef4ca824ee5854

commit a31644c6bbfce45f46d92f6bb8ef4ca824ee5854
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Thu Sep 28 21:58:34 2017

[Extensions] Enforce a maximum message size limit

Enforce a maximum message limit for extensions of a 64MB string (from
the JSON-ified object passed to the API call). We enforce a 128 MB limit
on IPC messages, but even that should be excessively large for our needs.
Also add metrics to see what average messages look like - 64MB is *huge*
(and we have to serialize/deserialize the object multiple times, so
message size can have a pretty big performance impact), so we may be
able to reduce this furter.

Add a test for the new behavior.

Bug:  766713 

Change-Id: Ic72b2c6c3ea3ca68a42fcd8061c9e006bc6f1795
Reviewed-on: https://chromium-review.googlesource.com/688674
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505176}
[modify] https://crrev.com/a31644c6bbfce45f46d92f6bb8ef4ca824ee5854/chrome/browser/extensions/extension_messages_apitest.cc
[add] https://crrev.com/a31644c6bbfce45f46d92f6bb8ef4ca824ee5854/chrome/test/data/extensions/api_test/messaging/large_messages/background.js
[add] https://crrev.com/a31644c6bbfce45f46d92f6bb8ef4ca824ee5854/chrome/test/data/extensions/api_test/messaging/large_messages/manifest.json
[modify] https://crrev.com/a31644c6bbfce45f46d92f6bb8ef4ca824ee5854/extensions/renderer/messaging_bindings.cc
[modify] https://crrev.com/a31644c6bbfce45f46d92f6bb8ef4ca824ee5854/extensions/renderer/messaging_bindings.h
[modify] https://crrev.com/a31644c6bbfce45f46d92f6bb8ef4ca824ee5854/extensions/renderer/resources/messaging.js
[modify] https://crrev.com/a31644c6bbfce45f46d92f6bb8ef4ca824ee5854/tools/metrics/histograms/histograms.xml

Automatically applying components based on information from OWNERS files. If this seems incorrect, please apply the Test-Predator-Wrong-Components label.

No reports since patch landed in 63.0.3227.0

https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.magic_signature_1.name%3D%27IPC%3A%3AChannelProxy%3A%3ASendInternal%27%20AND%20product.version%20%3E%3D%20%2763.0.3227.0%27%20AND%20%20custom_data.ChromeCrashProto.ptype%3D%27extension%27&sql_dialect=dremelsql&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D

I think this is fixed, but I'll continue to keep an eye on it.

Is this the primary cause of the IPC crashes in crbug.com/758902? Should this be merged?

crbug.com/758902 is about selecting text on the chrome://version page, which triggers a separate IPC.  I think these are separate issues, since this one is only about extensions sending massive messages (and other IPCs may need to be handled individually).

To be clear, bug 758902 was initiated by a generic IPC receiver stack trace which instigated this whole investigation, and it just happens that the chrome://version drag-and-drop behavior was a consistent way to repro the crash.

In hindsight with the data we have now (after moving to sender-side crashing) it seems likely that the majority of those crashes should be attributed to this bug instead. A merge may be desirable here, and it may also make sense for me to merge the sender-side crashing changes to M62 and M61, per bug 771235.

Just to update latest behavior, crashes are still observed on latest beta and dev channels.  

62.0.3202.45	15.45%	38 - Beta	
63.0.3230.0	1.22%	3  - Dev

Link to the list of the builds getting crash:
---------------------------------------------
https://crash.corp.google.com/browse?q=product.name%3D%27Chrome%27%20%20AND%20custom_data.ChromeCrashProto.ptype%3D%27renderer%27%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D%27IPC%3A%3AChannelProxy%3A%3ASendInternal%27&sql_dialect=dremelsql&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D#productversion:1000

Thanks!


	

Beta is 62, but the patch landed in 63.0.3227.0, so it's expected that we have crashes.

The crashes from 63.0.3230.0 are from different messages being sent.

https://crash.corp.google.com/browse?q=product.name%3D%27Chrome%27%20%20AND%20custom_data.ChromeCrashProto.ptype%3D%27renderer%27%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D%27IPC%3A%3AChannelProxy%3A%3ASendInternal%27%20AND%20product.version%3E%2763.0.3227.0%27&sql_dialect=dremelsql&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D

It looks like there's two from clipboard (maybe copying huge amounts of data?) and one from a console message (logging huge amounts of data?).  Ken, do those have separate bugs filed?

Not that I'm aware of, but they should

Also adding Merge-Request-62.  It seems like this does fix the extensions-related crash.  It's a bit late in the release cycle, so I'll leave it up to the release managers if this is worth merging.  So far, we've seen no issues with the patch, and it's a relatively small change.

This bug requires manual review: We are only 10 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Filed issue 772466 and issue 772468 for the clipboard and console bugs, respectively.

Since we're about 9 days away from Stable, recommendation is to wait until M63. Please confirm if you see any issues with this, or if there will be a large regression for users. 

From talking with Ken, there was a behavior change in M62 such that now these large messages crash the browser, instead of just exiting the renderer process.  Judging from numbers on Beta, it looks like this will be a significant crash on stable.  It seems like it might be worth merging.

The NextAction date has arrived: 2017-10-09

ClusterFuzz testcase 6310618306183168 is still reproducing on tip-of-tree build (trunk).

Please re-test your fix against this testcase and if the fix was incorrect or incomplete, please re-open the bug. Otherwise, ignore this notification and add ClusterFuzz-Wrong label.

@abdulsyed, please see comment #30.

https://clusterfuzz.com/v2/testcase-detail/6310618306183168 refers to a different crash (to do with blobs, rather than extension messages).

Thanks for more feedback. Approving merge to M62. Branch:3202

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6b618aa0771f9ec409919a96af5aa178e7fc91aa

commit 6b618aa0771f9ec409919a96af5aa178e7fc91aa
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Tue Oct 10 20:35:06 2017

[Extensions] Enforce a maximum message size limit

Enforce a maximum message limit for extensions of a 64MB string (from
the JSON-ified object passed to the API call). We enforce a 128 MB limit
on IPC messages, but even that should be excessively large for our needs.
Also add metrics to see what average messages look like - 64MB is *huge*
(and we have to serialize/deserialize the object multiple times, so
message size can have a pretty big performance impact), so we may be
able to reduce this furter.

Add a test for the new behavior.

Bug:  766713 

TBR=rdevlin.cronin@chromium.org

(cherry picked from commit a31644c6bbfce45f46d92f6bb8ef4ca824ee5854)

Change-Id: Ic72b2c6c3ea3ca68a42fcd8061c9e006bc6f1795
Reviewed-on: https://chromium-review.googlesource.com/688674
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#505176}
Reviewed-on: https://chromium-review.googlesource.com/710375
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#643}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/6b618aa0771f9ec409919a96af5aa178e7fc91aa/chrome/browser/extensions/extension_messages_apitest.cc
[add] https://crrev.com/6b618aa0771f9ec409919a96af5aa178e7fc91aa/chrome/test/data/extensions/api_test/messaging/large_messages/background.js
[add] https://crrev.com/6b618aa0771f9ec409919a96af5aa178e7fc91aa/chrome/test/data/extensions/api_test/messaging/large_messages/manifest.json
[modify] https://crrev.com/6b618aa0771f9ec409919a96af5aa178e7fc91aa/extensions/renderer/messaging_bindings.cc
[modify] https://crrev.com/6b618aa0771f9ec409919a96af5aa178e7fc91aa/extensions/renderer/messaging_bindings.h
[modify] https://crrev.com/6b618aa0771f9ec409919a96af5aa178e7fc91aa/extensions/renderer/resources/messaging.js
[modify] https://crrev.com/6b618aa0771f9ec409919a96af5aa178e7fc91aa/tools/metrics/histograms/histograms.xml

Please note that this change breaks useful extensions. It's unfortunate that it was merged back to M62. Please see Issue 774158.

rdevlin.cronin@, I'm still seeing considerable no. of crashes on Latest M62 Stable versions especially for "OS=Mac" after the above merge (c#36) to "3202 branch".

62.0.3202.94	32.40%	2099	
62.0.3202.89	21.89%	1418	
62.0.3202.75	17.80%	1153	
62.0.3202.62	19.59%	1269	

Ref link: https://goto.google.com/tisxf

Can you please take a look and comment what's happening here? We are doing M63 stable promotion in few days, so just wondering if it'll have potential impact on M63 as well?

Thanks in-advance!

@40 The reference link provided seems to be for a different crash?  Looks like it's for mojo::edk::NodeChannel::WriteChannelMessage(std::__1::unique_ptr<mojo::edk::Channel::Message, std::__1::default_delete<mojo::edk::Channel::Message> >) and issue 766032.

Yes, #40 is for crbug/766032 (which is duped into current crbug/766713).

Oh, I see.

I'm not sure that issue 766032 should have been duped into this - this was only a subset of the possible times when issue 766032 could have occurred.  Though this particular fix was merged to M62, there are other bugs (issue 772466 and issue 772468) where the crash can occur.

The reason the old stack trace is still showing up in M62 is that rockot's patch in 766032 was only in M63.  My suspicion is that the crashes we are seeing in M62 are from the other times this can happen (772466, 772468).

rockot@, would it make sense to un-dupe issue 766032 and mark it as blocked on the other bugs?

I don't think it makes sense to treat that as a meta-bug, as it can effectively never be closed, or will be re-opened ad infinitum whenever new message size violations are introduced.

I have however de-duped it from this bug and added a note for posterity.

rockot@, cool, thank you for the update!