Status Update
No update yet.
Comments
na...@google.com
[Empty comment from Monorail migration]
[Monorail components: Blink>MemoryAllocator]
[Monorail components: Blink>MemoryAllocator]
sa...@google.com
[Empty comment from Monorail migration]
kr...@chromium.org
Able to reproduce the issue on chrome latest stable #90.0.4430.85 using Windows 10, Mac 11.2.1 and Linux Debian Rodete by following steps as per https://crbug.com/chromium/1201109#c0 .
Note:Adding OS=Chrome and Android since this is Blink related
Reproducible
===========
92.0.4486.2 - Canary
92.0.4484.7 - Dev
91.0.4472.19- Beta
90.0.4430.85- Stable
Bisect Information:
----------------------------
Good Build: 89.0.4325.0
Bad Build: 89.0.4326.0
Change Log:
============
https://chromium.googlesource.com/chromium/src/+log/78ddb4f7c8d3a86095ba637f935154adb9a5c62b..4043bdca9f8cf37b04d4d7d3ab0a4018fc51a7c4
Suspecting below from the change log:
================================
Change-Id: I9e15d17dcfbe907ca5df8e00a7f77851cdc31e22
Reviewed-on:https://chromium-review.googlesource.com/c/chromium/src/+/2534370
bartekn@: Requesting you to please have a look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to owner concerned.
Thanks..!!
Note:Adding OS=Chrome and Android since this is Blink related
Reproducible
===========
92.0.4486.2 - Canary
92.0.4484.7 - Dev
91.0.4472.19- Beta
90.0.4430.85- Stable
Bisect Information:
----------------------------
Good Build: 89.0.4325.0
Bad Build: 89.0.4326.0
Change Log:
============
Suspecting below from the change log:
================================
Change-Id: I9e15d17dcfbe907ca5df8e00a7f77851cdc31e22
Reviewed-on:
bartekn@: Requesting you to please have a look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to owner concerned.
Thanks..!!
ba...@chromium.org
A limit for a single allocation in PartitionAlloc is 2GB. ArrayBuffers used PartitionAlloc for quite a while now. Handing over to Benoit, and CC'ing Chris, to see if the limit ought to be revised.
[Monorail components: -Blink>MemoryAllocator Blink>MemoryAllocator>Partition]
[Monorail components: -Blink>MemoryAllocator Blink>MemoryAllocator>Partition]
li...@chromium.org
Yes, I believe that the limit has been there for ArrayBuffer for a long time in Chrome.
We could lift this restriction, even though large allocations like that are likely to fail in many cases, on many clients. Meaning that your code has to be prepared for these allocations to fail. Sadly, much lower allocations do fail as well in a variety of cases.
However, this is not a simple constant change, since we would need to audit all the ArrayBuffer paths and make sure that we don't have any integer overflow hiding there. So it would take some time.
@reporter: Can you break down these allocations into smaller ones, and not have all memory allocated at all times? This would be required to work on 32 bit clients, and likely more or less required to work on many 64 bit ones as well, since Chrome limits the per-renderer memory footprint, and as a consequence, allocating several GB is likely to result in an Out Of Memory crash anyway.
We could lift this restriction, even though large allocations like that are likely to fail in many cases, on many clients. Meaning that your code has to be prepared for these allocations to fail. Sadly, much lower allocations do fail as well in a variety of cases.
However, this is not a simple constant change, since we would need to audit all the ArrayBuffer paths and make sure that we don't have any integer overflow hiding there. So it would take some time.
@reporter: Can you break down these allocations into smaller ones, and not have all memory allocated at all times? This would be required to work on 32 bit clients, and likely more or less required to work on many 64 bit ones as well, since Chrome limits the per-renderer memory footprint, and as a consequence, allocating several GB is likely to result in an Out Of Memory crash anyway.
iv...@gmail.com
All tools in my project are expecting single Uint8Array on the input, and I would like to keep it that way.
All I am asking is not to kill a website for asking a certain memory, when there is ten times that amount of memory free. Today, we have phones with 12 GB of RAM.
Could you do at least what Firefox does?
All I am asking is not to kill a website for asking a certain memory, when there is ten times that amount of memory free. Today, we have phones with 12 GB of RAM.
Could you do at least what Firefox does?
li...@chromium.org
Sorry about that. For context, there are a couple historical reasons for the current limits:
- Per-process limits: preventing sites from unknowingly bringing a machine to its knees. A single memory leak in an iframe somewhere can end up consuming multiple GiB, in which case it's likely better to crash the tab rather than rendering the machine unusable. That being said, on a 16GiB machine, the limit is pretty high (don't remember it of the top of my head, but should be large enough for you)
- Mitigating int overflow issues: there is a long history of "int"s being used for array sizes, leading to exploitable bugs.
The second one is the reason why we need to be careful with lifting the limit. I agree with you though, we should aim at raising the limit to allow powerful web applications to take advantage of it on machines with large amounts of memory.
- Per-process limits: preventing sites from unknowingly bringing a machine to its knees. A single memory leak in an iframe somewhere can end up consuming multiple GiB, in which case it's likely better to crash the tab rather than rendering the machine unusable. That being said, on a 16GiB machine, the limit is pretty high (don't remember it of the top of my head, but should be large enough for you)
- Mitigating int overflow issues: there is a long history of "int"s being used for array sizes, leading to exploitable bugs.
The second one is the reason why we need to be careful with lifting the limit. I agree with you though, we should aim at raising the limit to allow powerful web applications to take advantage of it on machines with large amounts of memory.
[Deleted User] <[Deleted User]>
This issue hasn't been updated in the last 30 days - please update it or consider lowering its priority.
Thanks for your time! To disable nags, add the Disable-Nags label.
For more details visithttps://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Thanks for your time! To disable nags, add the Disable-Nags label.
For more details visit
iv...@gmail.com
Can anyone fix it? As I said, I really need it for my app. I see no point in limiting the ArrayBuffer to 2 GB on devices with 32 GB of RAM or more. Even Firefox can do it.
pa...@chromium.org
In addition to the integer overflow risk (which we could solve with careful auditing and tests), it is also the case that some exploits require the attacker to, or work better when the attacker is able to, allocate large regions. We'd like to frustrate such attacks, to the extent possible.
We'll probably want to keep some limit. But if we can do the audit and add the tests, I can imagine that we could increase the ArrayBuffer size limit. The audit and the tests are a good idea anyway; fixes would be to use the right type everywhere (size_t, not int) and use base/numerics to make sure the arithmetic is safe.
We'll probably want to keep some limit. But if we can do the audit and add the tests, I can imagine that we could increase the ArrayBuffer size limit. The audit and the tests are a good idea anyway; fixes would be to use the right type everywhere (size_t, not int) and use base/numerics to make sure the arithmetic is safe.
ba...@google.com
There is one other reason preventing us from taking a "sky is the limit" approach, and it's the GigaCage. Pre-reserving larger address space for GigaCage incurs an actual commit charge cost on Windows <8.1, and we've seen actual OOM crashes from those systems.
Given everything that was said above, I think a 3~4GB limit isn't unreasonable.
Given everything that was said above, I think a 3~4GB limit isn't unreasonable.
ha...@chromium.org
I think we have to support 2+ GiB allocations for ArrayBuffers. There are legitimate use cases for it.
Do we need to support 4+ GiB allocations? If yes, we need to replace int32_t with size_t, in addition to the auditing and testing.
Do we need to support 4+ GiB allocations? If yes, we need to replace int32_t with size_t, in addition to the auditing and testing.
ha...@chromium.org
[Empty comment from Monorail migration]
iv...@gmail.com
Do you think you could fix it any time soon? There are too many people using Chrome and I have to recommend them to use Firefox: https://www.reddit.com/r/photopea/comments/o3qo40/cant_open_43200_x_21600_jpg/
yu...@chromium.org
[Empty comment from Monorail migration]
pa...@google.com
[Empty comment from Monorail migration]
[Deleted User] <[Deleted User]>
This issue hasn't been updated in the last 30 days - please update it or consider lowering its priority.
Thanks for your time! To disable nags, add the Disable-Nags label.
For more details visithttps://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Thanks for your time! To disable nags, add the Disable-Nags label.
For more details visit
iv...@gmail.com
Could you make Chrome allocate ArrayBuffers larger than 2.15 GB like Firefox can?
[Deleted User] <[Deleted User]>
This issue hasn't been updated in the last 30 days - please update it or consider lowering its priority.
Thanks for your time! To disable nags, add the Disable-Nags label.
For more details visithttps://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Thanks for your time! To disable nags, add the Disable-Nags label.
For more details visit
pa...@chromium.org
[Empty comment from Monorail migration]
[Deleted User] <[Deleted User]>
This issue hasn't been updated in the last 30 days - please update it or consider lowering its priority.
Thanks for your time! To disable nags, add the Disable-Nags label.
For more details visithttps://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Thanks for your time! To disable nags, add the Disable-Nags label.
For more details visit
iv...@gmail.com
Guys, can you fix it, please? I do not see any point in restricting users to 2 GB buffers, when there can be 16 or 32 GB of RAM available.
The volume of RAM in devices will probably keep growing, to hundreds of gigabytes in the future. So you will have to change it at some point anyway.
The volume of RAM in devices will probably keep growing, to hundreds of gigabytes in the future. So you will have to change it at some point anyway.
ba...@chromium.org
We're sorry for lack of updates. You're absolutely right, we'll have to change it at some point anyway, sooner than later. This is, however, not as easy as changing a constant -- see comments #7 and #10 about potential security implications, so it'll take some time to properly audit and harden the code. Updating the priority to reflect relative importance with what the team is currently working on.
ai...@gmail.com
I also ran into this limitation. For the traversal, I created a WebAssembly memory instance and passed it to the typed array constructor. WebAssembly memory supports up to 4 GB. Example:
// Allocate 65536 pages. 1 page == 64KB
const mem = new WebAssembly.Memory({initial: 65536})
// Pass the memory buffer to the Uint8Array constructor
const u8arr = new Uint8Array(mem.buffer)
// Allocate 65536 pages. 1 page == 64KB
const mem = new WebAssembly.Memory({initial: 65536})
// Pass the memory buffer to the Uint8Array constructor
const u8arr = new Uint8Array(mem.buffer)
ha...@google.com
We are actively working on the design to increase the buffer limit to 4 GB. It has a security implication and is likely to require a substantial amount of engineering work. We'll keep you posted on this bug!
ba...@chromium.org
[Empty comment from Monorail migration]
gi...@appspot.gserviceaccount.com
The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/fd3d255e519b9dbc696e0d175744df34fff640ae
commit fd3d255e519b9dbc696e0d175744df34fff640ae
Author: danakj <danakj@chromium.org>
Date: Thu Jan 06 18:12:36 2022
Add a GN config that warns about unsafe integer narrowing.
We currently build without -Wconversion (it's not part of -Wall, see
https://godbolt.org/z/aKbEracjK ), and with -Wno-narrowing and
-Wno-c++11-narrowing. This allows unsafe implicit narrowing conversions
or sign-mismatch comparisons that are likely to result in read/write
out of bounds, Undefined Behaviour, or other security breaches through
memory safety bugs.
Historically we mitigated against such bugs by artificially limiting
allocations to be < 2GB so that their sizes, indices, and offsets,
would fit within 31 bits. This allows an `int` to be misused but still
able to address the entire address space of any given allocation.
We would like to increase the allocation limit, which means we must
address the technical debt of these unsafe implicit conversions and
comparisons.
This CL introduces the prevent_unsafe_narrowing GN config, which when
enabled adds warnings to the GN target, which will produce errors in
problematic cases. Note that the config ignores the fact we also pass
-Wno-narrowing -Wno-c++11-narrowing, since -Wshorten-64-to-32 also
warns in the same scenarios, if narrowing a 64-bit integer to 32 bits
(seehttps://godbolt.org/z/Wxrvf7f6b ).
R=thakis@chromium.org
Bug: 1201109
Change-Id: I54d8f2a00191594dab0cc4f6d65c96b822a2f6c4
Reviewed-on:https://chromium-review.googlesource.com/c/chromium/src/+/3367413
Reviewed-by: Yuki Yamada <yukiy@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: danakj chromium <danakj@chromium.org>
Cr-Commit-Position: refs/heads/main@{#956170}
[modify]https://crrev.com/fd3d255e519b9dbc696e0d175744df34fff640ae/build/config/compiler/BUILD.gn
commit fd3d255e519b9dbc696e0d175744df34fff640ae
Author: danakj <danakj@chromium.org>
Date: Thu Jan 06 18:12:36 2022
Add a GN config that warns about unsafe integer narrowing.
We currently build without -Wconversion (it's not part of -Wall, see
-Wno-c++11-narrowing. This allows unsafe implicit narrowing conversions
or sign-mismatch comparisons that are likely to result in read/write
out of bounds, Undefined Behaviour, or other security breaches through
memory safety bugs.
Historically we mitigated against such bugs by artificially limiting
allocations to be < 2GB so that their sizes, indices, and offsets,
would fit within 31 bits. This allows an `int` to be misused but still
able to address the entire address space of any given allocation.
We would like to increase the allocation limit, which means we must
address the technical debt of these unsafe implicit conversions and
comparisons.
This CL introduces the prevent_unsafe_narrowing GN config, which when
enabled adds warnings to the GN target, which will produce errors in
problematic cases. Note that the config ignores the fact we also pass
-Wno-narrowing -Wno-c++11-narrowing, since -Wshorten-64-to-32 also
warns in the same scenarios, if narrowing a 64-bit integer to 32 bits
(see
R=thakis@chromium.org
Bug: 1201109
Change-Id: I54d8f2a00191594dab0cc4f6d65c96b822a2f6c4
Reviewed-on:
Reviewed-by: Yuki Yamada <yukiy@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: danakj chromium <danakj@chromium.org>
Cr-Commit-Position: refs/heads/main@{#956170}
[modify]
da...@chromium.org
[Empty comment from Monorail migration]
gi...@appspot.gserviceaccount.com
The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/b198fc2d402925f11cc86c5acb1a6084c2984ae1
commit b198fc2d402925f11cc86c5acb1a6084c2984ae1
Author: danakj <danakj@chromium.org>
Date: Thu Jan 13 20:46:02 2022
Avoid underflow if the relative_offset is a large negative number
If the relative_offset would produce a number less than 0, return
nullptr instead of wrapping around to return a potentially incorrect
sibling.
This came up when looking for bad unsigned comparisons.
Bug: 1201109
Change-Id: I9dee1357e6f8a7bbb0b8b5e5a4978c01cb433ebe
Reviewed-on:https://chromium-review.googlesource.com/c/chromium/src/+/3387086
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: danakj chromium <danakj@chromium.org>
Cr-Commit-Position: refs/heads/main@{#958808}
[modify]https://crrev.com/b198fc2d402925f11cc86c5acb1a6084c2984ae1/content/browser/renderer_host/render_frame_host_impl.cc
commit b198fc2d402925f11cc86c5acb1a6084c2984ae1
Author: danakj <danakj@chromium.org>
Date: Thu Jan 13 20:46:02 2022
Avoid underflow if the relative_offset is a large negative number
If the relative_offset would produce a number less than 0, return
nullptr instead of wrapping around to return a potentially incorrect
sibling.
This came up when looking for bad unsigned comparisons.
Bug: 1201109
Change-Id: I9dee1357e6f8a7bbb0b8b5e5a4978c01cb433ebe
Reviewed-on:
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: danakj chromium <danakj@chromium.org>
Cr-Commit-Position: refs/heads/main@{#958808}
[modify]
yu...@google.com
[Empty comment from Monorail migration]
yu...@google.com
[Empty comment from Monorail migration]
iv...@gmail.com
A user of my tool www.Photopea.com tried to do some work, which requires a lot of RAM (he has 64 GB of RAM). It crashed every time, so I told him to give it up, because no browser will probably give a website more than 8 - 10 GB of RAM. But he can still try it in a different browser.
Now, he wrote me, that he tried it in Firefox, and it worked. And I am shocked.https://i.imgur.com/JROpc67.png
Then, I remembered this issue, which is probably the reason it crashed in Chrome. Guys, please, put more effort into fixing it, it is really important. I really appreciate your work!
Now, he wrote me, that he tried it in Firefox, and it worked. And I am shocked.
Then, I remembered this issue, which is probably the reason it crashed in Chrome. Guys, please, put more effort into fixing it, it is really important. I really appreciate your work!
ba...@chromium.org
[Empty comment from Monorail migration]
al...@gmail.com
@yukiy can you please transfer the ownership to someones before you leave?
ba...@chromium.org
[Empty comment from Monorail migration]
ba...@chromium.org
Re https://crbug.com/chromium/1201109#c32 : Thanks for continuously cheering for us. Our security team concluded that simply increasing the allocation limit would be too dangerous, until we first get rid of unsafe integer conversions or truncations. This requires work on the entire codebase, so it'll take some time. Please follow the blocking issues for progress updates.
ar...@etoro.com
Regarding #33, while https://crbug.com/chromium/1305831 (https://bugs.chromium.org/p/chromium/issues/detail?id=1305831 ), which has been merged into this issue, is related, I don't think it's a duplicate, as its essence is the ability to detect, debug and diagnose similar issues that are caused by a bug in the client's code (a memory leak), and not about the need to increase the memory from 2GB to 64GB. Please reopen https://crbug.com/chromium/1305831 and address it accordingly. Thanks!
gi...@appspot.gserviceaccount.com
The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/6a1d746dafa867af9078367317779e2789d9217c
commit 6a1d746dafa867af9078367317779e2789d9217c
Author: Andreas Haas <ahaas@chromium.org>
Date: Fri Apr 29 18:09:02 2022
[arraybuffer] Protect Blink from 2+GB ArrayBuffers
R=haraken@chromium.org
CC=clemensb@chromium.org, danakj@chromium.org
Bug: chromium:1201109
Change-Id: I8f393957e166c3fc4a6a5cb5e56cfec539a61511
Reviewed-on:https://chromium-review.googlesource.com/c/chromium/src/+/3602782
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#997800}
[modify]https://crrev.com/6a1d746dafa867af9078367317779e2789d9217c/third_party/blink/public/mojom/web_feature/web_feature.mojom
[modify]https://crrev.com/6a1d746dafa867af9078367317779e2789d9217c/third_party/blink/renderer/bindings/core/v8/native_value_traits_buffer_sources.cc
[modify]https://crrev.com/6a1d746dafa867af9078367317779e2789d9217c/tools/metrics/histograms/enums.xml
commit 6a1d746dafa867af9078367317779e2789d9217c
Author: Andreas Haas <ahaas@chromium.org>
Date: Fri Apr 29 18:09:02 2022
[arraybuffer] Protect Blink from 2+GB ArrayBuffers
R=haraken@chromium.org
CC=clemensb@chromium.org, danakj@chromium.org
Bug: chromium:1201109
Change-Id: I8f393957e166c3fc4a6a5cb5e56cfec539a61511
Reviewed-on:
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#997800}
[modify]
[modify]
[modify]
gi...@appspot.gserviceaccount.com
The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/9be133e40586ca7c170ed539fc5680a829d6bcff
commit 9be133e40586ca7c170ed539fc5680a829d6bcff
Author: Andreas Haas <ahaas@chromium.org>
Date: Sat Apr 30 11:46:17 2022
Revert "[arraybuffer] Protect Blink from 2+GB ArrayBuffers"
This reverts commit 6a1d746dafa867af9078367317779e2789d9217c.
Reason for revert: Breaks Photoshop
Original change's description:
commit 9be133e40586ca7c170ed539fc5680a829d6bcff
Author: Andreas Haas <ahaas@chromium.org>
Date: Sat Apr 30 11:46:17 2022
Revert "[arraybuffer] Protect Blink from 2+GB ArrayBuffers"
This reverts commit 6a1d746dafa867af9078367317779e2789d9217c.
Reason for revert: Breaks Photoshop
Original change's description:
Bug: chromium:1201109
Change-Id: I79cba720a921383c64da6d78a6e1875e045ea3d1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on:
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Auto-Submit: Andreas Haas <ahaas@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/main@{#998084}
[modify]
[modify]
[modify]
gi...@appspot.gserviceaccount.com
The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/c324a89710943b2544b9049767225e337fb1e6b5
commit c324a89710943b2544b9049767225e337fb1e6b5
Author: Andreas Haas <ahaas@chromium.org>
Date: Thu May 05 05:53:55 2022
Reland "[arraybuffer] Protect Blink from 2+GB ArrayBuffers"
The original CL broke postmessage of bounds checks, which was used by
Photoshop to share WebAssembly memory to workers. This CL adds an
additional function that does not do the bounds check on ArrayBuffers.
R=haraken@chromium.org, adamk@chromium.org
CC=clemensb@chromium.org, danakj@chromium.org
Bug: chromium:1201109
Change-Id: I0655746061dddbbb448abb28a41f46d54f0e8e57
Reviewed-on:https://chromium-review.googlesource.com/c/chromium/src/+/3620333
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/main@{#999760}
[add]https://crrev.com/c324a89710943b2544b9049767225e337fb1e6b5/third_party/blink/web_tests/wpt_internal/wasm/postmessage_big_wasm_memory.https.html
[modify]https://crrev.com/c324a89710943b2544b9049767225e337fb1e6b5/third_party/blink/public/mojom/web_feature/web_feature.mojom
[modify]https://crrev.com/c324a89710943b2544b9049767225e337fb1e6b5/third_party/blink/renderer/bindings/core/v8/native_value_traits_buffer_sources.cc
[add]https://crrev.com/c324a89710943b2544b9049767225e337fb1e6b5/third_party/blink/web_tests/fast/arraybuffer/size_check_of_web_api.html
[add]https://crrev.com/c324a89710943b2544b9049767225e337fb1e6b5/third_party/blink/web_tests/wpt_internal/wasm/postmessage_big_wasm_memory.https.html.headers
[modify]https://crrev.com/c324a89710943b2544b9049767225e337fb1e6b5/tools/metrics/histograms/enums.xml
[modify]https://crrev.com/c324a89710943b2544b9049767225e337fb1e6b5/third_party/blink/renderer/bindings/core/v8/serialization/v8_script_value_serializer.cc
commit c324a89710943b2544b9049767225e337fb1e6b5
Author: Andreas Haas <ahaas@chromium.org>
Date: Thu May 05 05:53:55 2022
Reland "[arraybuffer] Protect Blink from 2+GB ArrayBuffers"
The original CL broke postmessage of bounds checks, which was used by
Photoshop to share WebAssembly memory to workers. This CL adds an
additional function that does not do the bounds check on ArrayBuffers.
R=haraken@chromium.org, adamk@chromium.org
CC=clemensb@chromium.org, danakj@chromium.org
Bug: chromium:1201109
Change-Id: I0655746061dddbbb448abb28a41f46d54f0e8e57
Reviewed-on:
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/main@{#999760}
[add]
[modify]
[modify]
[add]
[add]
[modify]
[modify]
gi...@appspot.gserviceaccount.com
The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/2441efc1406fa02b334fad353bcb19b5de6f3e87
commit 2441efc1406fa02b334fad353bcb19b5de6f3e87
Author: Meredith Lane <meredithl@chromium.org>
Date: Thu May 05 07:55:06 2022
Revert "Reland "[arraybuffer] Protect Blink from 2+GB ArrayBuffers""
This reverts commit c324a89710943b2544b9049767225e337fb1e6b5.
Reason for revert: Fails on Win7 build starting fromhttps://ci.chromium.org/ui/p/chromium/builders/ci/Win7%20Tests%20(1)/127822/overview
Original change's description:
commit 2441efc1406fa02b334fad353bcb19b5de6f3e87
Author: Meredith Lane <meredithl@chromium.org>
Date: Thu May 05 07:55:06 2022
Revert "Reland "[arraybuffer] Protect Blink from 2+GB ArrayBuffers""
This reverts commit c324a89710943b2544b9049767225e337fb1e6b5.
Reason for revert: Fails on Win7 build starting from
Original change's description:
Bug: chromium:1201109
Change-Id: I6ffa3adcf5615fac0f3fdfd7c011e309caae5152
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on:
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Owners-Override: Meredith Lane <meredithl@chromium.org>
Auto-Submit: Meredith Lane <meredithl@chromium.org>
Reviewed-by: Meredith Lane <meredithl@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Meredith Lane <meredithl@chromium.org>
Cr-Commit-Position: refs/heads/main@{#999785}
[delete]
[modify]
[modify]
[delete]
[delete]
[modify]
[modify]
gi...@appspot.gserviceaccount.com
The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/37c9cdad584891bd057ced018531840078899ae5
commit 37c9cdad584891bd057ced018531840078899ae5
Author: Andreas Haas <ahaas@chromium.org>
Date: Fri May 06 04:30:34 2022
Reland^2 "[arraybuffer] Protect Blink from 2+GB ArrayBuffers"
The tests in the original CL allocated big amounts of memory, which
failed on some bots. The new CL checks if allocation succeeded or not.
Bug: chromium:1201109
Change-Id: Ib6bb6938e869788d0939fd9f66cba2003fc3efd1
Reviewed-on:https://chromium-review.googlesource.com/c/chromium/src/+/3627585
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1000244}
[add]https://crrev.com/37c9cdad584891bd057ced018531840078899ae5/third_party/blink/web_tests/wpt_internal/wasm/postmessage_big_wasm_memory.https.html
[delete]https://crrev.com/3a0fe2618bd633ea42a1c7ab9377cc5a8f268425/third_party/blink/web_tests/platform/win/wpt_internal/wasm/postmessage_big_wasm_memory.https-expected.txt
[modify]https://crrev.com/37c9cdad584891bd057ced018531840078899ae5/third_party/blink/public/mojom/web_feature/web_feature.mojom
[modify]https://crrev.com/37c9cdad584891bd057ced018531840078899ae5/third_party/blink/renderer/bindings/core/v8/native_value_traits_buffer_sources.cc
[add]https://crrev.com/37c9cdad584891bd057ced018531840078899ae5/third_party/blink/web_tests/fast/arraybuffer/size_check_of_web_api.html
[add]https://crrev.com/37c9cdad584891bd057ced018531840078899ae5/third_party/blink/web_tests/wpt_internal/wasm/postmessage_big_wasm_memory.https.html.headers
[modify]https://crrev.com/37c9cdad584891bd057ced018531840078899ae5/tools/metrics/histograms/enums.xml
[modify]https://crrev.com/37c9cdad584891bd057ced018531840078899ae5/third_party/blink/renderer/bindings/core/v8/serialization/v8_script_value_serializer.cc
commit 37c9cdad584891bd057ced018531840078899ae5
Author: Andreas Haas <ahaas@chromium.org>
Date: Fri May 06 04:30:34 2022
Reland^2 "[arraybuffer] Protect Blink from 2+GB ArrayBuffers"
The tests in the original CL allocated big amounts of memory, which
failed on some bots. The new CL checks if allocation succeeded or not.
Bug: chromium:1201109
Change-Id: Ib6bb6938e869788d0939fd9f66cba2003fc3efd1
Reviewed-on:
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1000244}
[add]
[delete]
[modify]
[modify]
[add]
[add]
[modify]
[modify]
ms...@chromium.org
[Empty comment from Monorail migration]
gi...@appspot.gserviceaccount.com
The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/042ad65fa4ee3883a73445de66d446f258b2ffa7
commit 042ad65fa4ee3883a73445de66d446f258b2ffa7
Author: Kenneth Russell <kbr@chromium.org>
Date: Wed Jun 15 02:45:05 2022
Guard against too-large arrays passed to uniform*v.
Catch situations where computing the byte size of the incoming array
would overflow.
Beyond this, detect failures to allocate space in CommandBufferHelper.
In this situation, persistently lose the context client-side. Remove
DCHECKs preventing these guards from being reached.
Add new feature to disable ArrayBuffer size restrictions for testing.
Use it in the new test; it is required for it to pass.
Bug: 1316368
Bug: 1201109
Change-Id: Ic345aed296e2d9a3f44e33c4c207e12b64c4f312
Reviewed-on:https://chromium-review.googlesource.com/c/chromium/src/+/3688833
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Derek Schuff <dschuff@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Gregg Tavares <gman@chromium.org>
Reviewed-by: Victor Miura <vmiura@chromium.org>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/heads/main@{#1014267}
[modify]https://crrev.com/042ad65fa4ee3883a73445de66d446f258b2ffa7/gpu/ipc/client/command_buffer_proxy_impl.h
[modify]https://crrev.com/042ad65fa4ee3883a73445de66d446f258b2ffa7/ppapi/proxy/ppapi_command_buffer_proxy.cc
[modify]https://crrev.com/042ad65fa4ee3883a73445de66d446f258b2ffa7/third_party/blink/public/common/features.h
[modify]https://crrev.com/042ad65fa4ee3883a73445de66d446f258b2ffa7/third_party/blink/renderer/bindings/core/v8/native_value_traits_buffer_sources.cc
[modify]https://crrev.com/042ad65fa4ee3883a73445de66d446f258b2ffa7/gpu/command_buffer/client/client_test_helper.cc
[modify]https://crrev.com/042ad65fa4ee3883a73445de66d446f258b2ffa7/third_party/blink/common/features.cc
[add]https://crrev.com/042ad65fa4ee3883a73445de66d446f258b2ffa7/content/test/data/gpu/webgl-overly-large-uniform.html
[modify]https://crrev.com/042ad65fa4ee3883a73445de66d446f258b2ffa7/gpu/command_buffer/service/command_buffer_direct.cc
[modify]https://crrev.com/042ad65fa4ee3883a73445de66d446f258b2ffa7/gpu/command_buffer/client/cmd_buffer_helper.cc
[modify]https://crrev.com/042ad65fa4ee3883a73445de66d446f258b2ffa7/gpu/ipc/client/command_buffer_proxy_impl.cc
[modify]https://crrev.com/042ad65fa4ee3883a73445de66d446f258b2ffa7/content/test/gpu/gpu_tests/context_lost_integration_test.py
[modify]https://crrev.com/042ad65fa4ee3883a73445de66d446f258b2ffa7/third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.cc
[modify]https://crrev.com/042ad65fa4ee3883a73445de66d446f258b2ffa7/gpu/command_buffer/client/client_discardable_manager_unittest.cc
[modify]https://crrev.com/042ad65fa4ee3883a73445de66d446f258b2ffa7/gpu/command_buffer/service/command_buffer_direct.h
[modify]https://crrev.com/042ad65fa4ee3883a73445de66d446f258b2ffa7/gpu/command_buffer/common/command_buffer.h
[modify]https://crrev.com/042ad65fa4ee3883a73445de66d446f258b2ffa7/gpu/ipc/in_process_command_buffer.h
[modify]https://crrev.com/042ad65fa4ee3883a73445de66d446f258b2ffa7/gpu/command_buffer/client/client_test_helper.h
[modify]https://crrev.com/042ad65fa4ee3883a73445de66d446f258b2ffa7/ppapi/proxy/ppapi_command_buffer_proxy.h
[modify]https://crrev.com/042ad65fa4ee3883a73445de66d446f258b2ffa7/gpu/ipc/in_process_command_buffer.cc
commit 042ad65fa4ee3883a73445de66d446f258b2ffa7
Author: Kenneth Russell <kbr@chromium.org>
Date: Wed Jun 15 02:45:05 2022
Guard against too-large arrays passed to uniform*v.
Catch situations where computing the byte size of the incoming array
would overflow.
Beyond this, detect failures to allocate space in CommandBufferHelper.
In this situation, persistently lose the context client-side. Remove
DCHECKs preventing these guards from being reached.
Add new feature to disable ArrayBuffer size restrictions for testing.
Use it in the new test; it is required for it to pass.
Bug: 1316368
Bug: 1201109
Change-Id: Ic345aed296e2d9a3f44e33c4c207e12b64c4f312
Reviewed-on:
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Derek Schuff <dschuff@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Gregg Tavares <gman@chromium.org>
Reviewed-by: Victor Miura <vmiura@chromium.org>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/heads/main@{#1014267}
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[add]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
kb...@chromium.org
[Empty comment from Monorail migration]
kb...@chromium.org
[Empty comment from Monorail migration]
kb...@chromium.org
[Empty comment from Monorail migration]
iv...@gmail.com
Is there any progress on this issue?
ba...@chromium.org
I'm sorry for the lack of updates. Yes, there has been some progress, but I am afraid not as much as we'd hope for.
We tried going in the direction that our security team proposed, that'd allow us to raise the allocator limit across the board. Upon attempting to implement, we found we needed to reevaluate what we can feasibly accomplish. You can read more details herehttps://docs.google.com/document/d/1CEOpqW2dA_zxAxpi8n6CuXcmx6jByRxqnSNAcLGzN04/edit
We did, however, reach a viable compromise that will allow us to raise the allocator limit specific to ArrayBuffers, which is what you care about. But first we'll have to harden the code around it. Here is the action plan that we follow:
1) Land the 2 GB limit check to the V8 -> C++ conversion layer.
2) Land the 2 GB limit check to places where C++ allocates ArrayBuffers.
3) Increase the memory limit of PA's ArrayBuffer partition.
1) has already been completed. 2) will happen no earlier than in Q1. 3) should be easy once the other two are completed.
We tried going in the direction that our security team proposed, that'd allow us to raise the allocator limit across the board. Upon attempting to implement, we found we needed to reevaluate what we can feasibly accomplish. You can read more details here
We did, however, reach a viable compromise that will allow us to raise the allocator limit specific to ArrayBuffers, which is what you care about. But first we'll have to harden the code around it. Here is the action plan that we follow:
1) Land the 2 GB limit check to the V8 -> C++ conversion layer.
2) Land the 2 GB limit check to places where C++ allocates ArrayBuffers.
3) Increase the memory limit of PA's ArrayBuffer partition.
1) has already been completed. 2) will happen no earlier than in Q1. 3) should be easy once the other two are completed.
je...@changehealthcare.com
I know it's odd and hacky, but you/we can allocated bigger arrayBuffers with WASM memory:
memory = new WebAssembly.Memory({initial: 60000,maximum: 60000});
You can even create sharedArrayBuffers in this way and you can grow them in place. 🤯
It's a bit of an odd workaround, but given it exists the security arguments in the linked document feels a bit moot?
memory = new WebAssembly.Memory({initial: 60000,maximum: 60000});
You can even create sharedArrayBuffers in this way and you can grow them in place. 🤯
It's a bit of an odd workaround, but given it exists the security arguments in the linked document feels a bit moot?
da...@chromium.org
> 1) Land the 2 GB limit check to the V8 -> C++ conversion layer.
This keeps the large array buffers from leaking from Wasm into C++ code.
This keeps the large array buffers from leaking from Wasm into C++ code.
je...@changehealthcare.com
I am not following. If I can:
const memory = new WebAssembly.Memory({initial: 60000,maximum: 60000});
const a = new Uint8Array(memory.buffer)
an use it happily (and presumably safely), why can't I
const b = new Uint8Array(3932160000) // Uncaught RangeError: Array buffer allocation failed
Obviously the plumbing is there to support 4GB ArrayBuffers since WASM added support for 4GB heap?
const memory = new WebAssembly.Memory({initial: 60000,maximum: 60000});
const a = new Uint8Array(memory.buffer)
an use it happily (and presumably safely), why can't I
const b = new Uint8Array(3932160000) // Uncaught RangeError: Array buffer allocation failed
Obviously the plumbing is there to support 4GB ArrayBuffers since WASM added support for 4GB heap?
ha...@google.com
[Empty comment from Monorail migration]
ha...@google.com
[Empty comment from Monorail migration]
ha...@google.com
[Empty comment from Monorail migration]
is...@google.com
This issue was migrated from crbug.com/chromium/1201109?no_tracker_redirect=1
[Monorail blocked-on:crbug.com/chromium/1287175 , crbug.com/chromium/1289256 , crbug.com/chromium/1292951 , crbug.com/chromium/1316368 , crbug.com/chromium/1321364 , crbug.com/chromium/1344047 , crbug.com/chromium/1356246 ]
[Monorail mergedwith:crbug.com/chromium/1213384 , crbug.com/chromium/1305831 ]
[Monorail components added to Component Tags custom field.]
[Monorail blocked-on:
[Monorail mergedwith:
[Monorail components added to Component Tags custom field.]
iv...@gmail.com
Are there any news? I still have to tell people to use my webapp in Firefox when they need to open "too large" files :(
cl...@chromium.org
The limit in V8 has been bumped to 32GB in https://crbug.com/v8/4153 . This was released in M-119.
Can you verify if this works for your webapp now?
Can you verify if this works for your webapp now?
iv...@gmail.com
In my bug report, I was talking about this demo: https://jsfiddle.net/18vj3u5d/ . It works only in Firefox, not in Chrome. Are you able to run it in Chrome?
cl...@chromium.org
It works in d8, but not in chrome, so I guess we still need to bump the PartitionAlloc limit.
ba...@google.com
Thanks for continuous care, Ivan. Can you try the suggestion in
iv...@gmail.com
I do not know much about Webassembly, I am not using Webassembly in this case. So I dont know what the "suggestion" means :(
I need to allocate a long Uint8Array to hold the color values of an image.
I need to allocate a long Uint8Array to hold the color values of an image.
ml...@chromium.org
With
ba...@google.com
I do not know much about Webassembly, I am not using Webassembly in this case. So I dont know what the "suggestion" means :(
I should've pointed to
With
we can effectively create an Uint8Array that can e.g. be passed into the Blob API. If those checks are not already in place then it's already possible to leak such buffers into C++, no? #comment51
To clarify my understanding (and I hope security folks here can correct me if I'm wrong), I believe new Uint8Array(2146*1000*1000); and const memory = new WebAssembly.Memory({initial: 32746,maximum: 32746}); new Uint8Array(memory.buffer); are equivalent in terms of the security risk. IIUC work done in
The remaining difference now is the implementation complexity. The former method operates on page granularity, hence simply maps in entire pages, which is trivial. The latter operates on byte granularity, hence uses a memory allocator (PartitionAlloc), which would be more complicated to adapt to >2GB allocations. Also I should mention that PartitionAlloc is used for multiple other purposes, and we definitely don't want to increase the 2GB limit there, so that creates an additional complication.
iv...@gmail.com
That is really interesting. The only prolbem I see now is, that the ArrayBuffer size is always a multiple of 65536, and even though I can create a Uint8Array of any length on top of it, I am not sure if all places of my code can handle the ArrayBuffer being a different size than the Uint8Array.
But if it is so simple, why cant you make the new Uint8Array(...) work? It works fine in Firefox.
But if it is so simple, why cant you make the new Uint8Array(...) work? It works fine in Firefox.
Description
I have 24 GB of RAM, 16 GB of which is free.
I am developing a photo editor
This works very well in Firefox. I think it used to work in previous versions of Chrome, too.