Status Update
Comments
cl...@appspot.gserviceaccount.com <cl...@appspot.gserviceaccount.com> #2
th...@chromium.org <th...@chromium.org> #3
It doesn't look like ClusterFuzz is having luck with this. Triaging this speculatively, matching the high severity from
I'm also setting the FoundIn speculatively to extended stable (M124).
amaiorano@: This looks similar to some other issues you've looked into recently. Could you PTAL at this issue? Note I am tagging you on two other similar ones as well. Could you please update the FoundIn if you learn that this was introduced more recently than M124?
th...@chromium.org <th...@chromium.org>
24...@project.gserviceaccount.com <24...@project.gserviceaccount.com> #4
pe...@google.com <pe...@google.com> #5
pe...@google.com <pe...@google.com> #6
am...@google.com <am...@google.com> #7
-
Cannot reproduce the crash when opening the attached
indexAddToListUAF.html
in Chrome; however, as the bug states, this is likely because my GPU does not support Shader Model 6.6 - mine supports SM 6.5. -
I am able to reproduce the ASAN failure when running an asan build of dxc against the attached
standalone.hlsl
and get a similar ASAN output with-T cs_6_6
, but dxc succeeds when compiling with-T cs_6_5
(or less), so this is specific to the SM 6.6 path in DXC. -
From my ASAN output, we're hitting a heap-use-after-free on an allocation of a
PHINode
that was created inSimplifyCFG
inSimplifyCondBranchToCondBranch
, then later deleted inSimplifyCFG
inSimplifyTerminatorOnSelect
, and then accessed in the same function. So this all happens inSimplifyCFG
, with the delete and use-after-free happening inSimplifyTerminatorOnSelect
.
Needs more investigation.
am...@google.com <am...@google.com> #8
Potential workaround: as we don't currently use any shader model 6.6 features yet, Dawn could limit the shader model to 6.5.
am...@google.com <am...@google.com> #9
In attempting to simplify the standalone.hlsl
file, very few changes are required to make the ASAN error disappear, but instead emit an assertion. Assuming the assert is related, I've reduced the code to a bare minimum that still reproduces the assertion:
[numthreads(1, 1, 1)]
void main() {
int i = 0;
while (true) {
for (i = 0; i < 2; i++) {
while (true) {
int unused = 0;
while (true) {
if (i < 2) {
return;
} else {
break;
}
}
if (i < 2) { break; }
}
}
}
}
And the error:
$ ./bin/dxc -T cs_6_6 standalone_reduced.hlsl
dxc: /home/amaiorano/src/external/DirectXShaderCompiler/include/llvm/IR/CFG.h:216: Self &llvm::SuccIterator<llvm::TerminatorInst *, llvm::BasicBlock>::operator+=(int) [Term_ = llvm::TerminatorInst *, BB_ = llvm::BasicBlock]: Assertion `index_is_valid(new_idx) && "Iterator index out of bound"' failed.
Aborted
am...@google.com <am...@google.com> #13
Upstream fix was
am...@google.com <am...@google.com> #14
I tested this on my personal PC since I needed a machine with a GPU that supports Shader Model 6.6.
-
I tested using regular Chrome, Version 124.0.6367.208 (Official Build) (64-bit), and reproduced the GPU process crash when opening the attached
indexAddToListUAF.html
, as expected. -
I tested using latest Canary, Version 127.0.6492.0 (Official Build) canary (64-bit), and the crash no longer reproduces when opening the attached
indexAddToListUAF.html
.
This is the upstream fix in our DXC mirror:
This is the roll of DXC with that fix into Dawn:
This is the roll of Dawn with that fix into Chrome:
pe...@google.com <pe...@google.com> #15
This is sufficiently serious that it should be merged to stable. But I can't see a Chromium repo commit here,so you will need to investigate what - if anything - needs to be merged to M125. Is there a fix in some other repo which should be merged? Or, perhaps this ticket is a duplicate of some other ticket which has the real fix: please track that down and ensure it is merged appropriately.
This is sufficiently serious that it should be merged to beta. But I can't see a Chromium repo commit here,so you will need to investigate what - if anything - needs to be merged to M126. Is there a fix in some other repo which should be merged? Or, perhaps this ticket is a duplicate of some other ticket which has the real fix: please track that down and ensure it is merged appropriately.
Merge review required: no relevant commits could be automatically detected (via Git Watcher comments), sending to merge review for manual evaluation. If you have not already manually listed the relevant commits to be merged via a comment above, please do so ASAP.
Merge review required: no relevant commits could be automatically detected (via Git Watcher comments), sending to merge review for manual evaluation. If you have not already manually listed the relevant commits to be merged via a comment above, please do so ASAP.
Merge review required: no relevant commits could be automatically detected (via Git Watcher comments), sending to merge review for manual evaluation. If you have not already manually listed the relevant commits to be merged via a comment above, please do so ASAP.
Thank you for fixing this security bug! We aim to ship security fixes as quickly as possible, to limit their opportunity for exploitation as an "n-day" (that is, a bug where git fixes are developed into attacks before those fixes reach users).
We have determined this fix is necessary on milestone(s): [124, 125, 126].
Please answer the following questions so that we can safely process this merge request:
1. Which CLs should be backmerged? (Please include Gerrit links.)
2. Has this fix been verified on Canary to not pose any stability regressions?
3. Does this fix pose any potential non-verifiable stability risks?
4. Does this fix pose any known compatibility risks?
5. Does it require manual verification by the test team? If so, please describe required testing.
am...@google.com <am...@google.com> #16
- Which CLs should be backmerged? (Please include Gerrit links.)
This is the DXC -> Dawn CL that rolls in the fix:
This is the Dawn -> Chromium CL:
But we cannot cherry-pick these. I will need to manually cherry-pick the fix from the DXC mirror into Dawn's chromium release branches.
- Has this fix been verified on Canary to not pose any stability regressions?
Yes.
- Does this fix pose any potential non-verifiable stability risks?
No.
- Does this fix pose any known compatibility risks?
No.
- Does it require manual verification by the test team? If so, please describe required testing.
Can be tested on latest Canary on a Windows machine with a GPU that supports Shader Model 6.6 by opening the attached indexAddToListUAF.html
and ensuring that the GPU process does not crash.
sr...@google.com <sr...@google.com> #17
am...@chromium.org <am...@chromium.org> #18
I've review data on Canary on which this roll was landed:
- M126 Beta / branch 6478
- M125 Stable / branch 6422
- M124 Extended Stable / branch 6367
as soon as possible, before 10am Pacific Time on Tuesday, 28 May (reminder that Monday is a US holiday, so merge by EOD tomorrow / Friday is preferred)
ap...@google.com <ap...@google.com> #19
Branch: refs/branch-heads/patches/6478
commit 6a6ff9dafe27caebfc47bace6c3a029bb42007f3
Author: Antonio Maiorano <amaiorano@google.com>
Date: Mon May 27 15:43:52 2024
Fix use-after-free in SimplifyCFG (#6628)
When SimplifySwitchOnSelect calls SimplifyTerminatorOnSelect, it holds
onto the select's condition value to use for the conditional branch it
replaces the switch with. When removing the switch's unused
predecessors, it must make sure not to delete PHIs in case one of them
is used by the condition value, otherwise the condition value itself may
get deleted, resulting in an use-after-free.
Note that this was fixed in LLVM as well:
Bug: chromium:338103465
Change-Id: I0f1870f1f84b2ce14a1bc883e3d2f6086ee14013
Reviewed-on:
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Natalie Chouinard <chouinard@chromium.org>
M lib/Transforms/Utils/SimplifyCFG.cpp
A tools/clang/test/DXC/Passes/SimplifyCFG/simplifycfg-uaf-select-condition.ll
ap...@google.com <ap...@google.com> #20
Branch: refs/branch-heads/patches/6422
commit e3dec39021a438ef070a1f7dbbef8760f358cb33
Author: Antonio Maiorano <amaiorano@google.com>
Date: Mon May 27 15:43:03 2024
Fix use-after-free in SimplifyCFG (#6628)
When SimplifySwitchOnSelect calls SimplifyTerminatorOnSelect, it holds
onto the select's condition value to use for the conditional branch it
replaces the switch with. When removing the switch's unused
predecessors, it must make sure not to delete PHIs in case one of them
is used by the condition value, otherwise the condition value itself may
get deleted, resulting in an use-after-free.
Note that this was fixed in LLVM as well:
Bug: chromium:338103465
Change-Id: I06521a6ef02774c81cfebcc9d5a0610c0c95e7b5
Reviewed-on:
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Natalie Chouinard <chouinard@chromium.org>
M lib/Transforms/Utils/SimplifyCFG.cpp
A tools/clang/test/DXC/Passes/SimplifyCFG/simplifycfg-uaf-select-condition.ll
ap...@google.com <ap...@google.com> #21
Branch: refs/branch-heads/patches/6367
commit 511cfef8e0509d172fbfa156be8a97ed2b42590b
Author: Antonio Maiorano <amaiorano@google.com>
Date: Mon May 27 15:41:40 2024
Fix use-after-free in SimplifyCFG (#6628)
When SimplifySwitchOnSelect calls SimplifyTerminatorOnSelect, it holds
onto the select's condition value to use for the conditional branch it
replaces the switch with. When removing the switch's unused
predecessors, it must make sure not to delete PHIs in case one of them
is used by the condition value, otherwise the condition value itself may
get deleted, resulting in an use-after-free.
Note that this was fixed in LLVM as well:
Bug: chromium:338103465
Change-Id: Iff5d5f2e3ecf38a3fb22bbc65e7c33ad0de659fb
Reviewed-on:
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Natalie Chouinard <chouinard@chromium.org>
M lib/Transforms/Utils/SimplifyCFG.cpp
A tools/clang/test/DXC/Passes/SimplifyCFG/simplifycfg-uaf-select-condition.ll
ap...@google.com <ap...@google.com> #22
Branch: chromium/6478
commit 4c6214625c4fd6d38b1604af39ce64395358ca55
Author: Antonio Maiorano <amaiorano@google.com>
Date: Mon May 27 20:28:52 2024
DEPS: Update DXC to patched branch
Bug: chromium:338103465
Change-Id: I1372195c5f52e5e632dc7a34ec1c9f14e3fa1078
Reviewed-on:
Reviewed-by: Natalie Chouinard <chouinard@google.com>
M DEPS
M third_party/dxc
ap...@google.com <ap...@google.com> #23
Branch: chromium/6422
commit fe3821ae5458d9a1a67c2e6a0f19aee8d6f90322
Author: Antonio Maiorano <amaiorano@google.com>
Date: Mon May 27 20:28:46 2024
DEPS: Update DXC to patched branch
Bug: chromium:338103465
Change-Id: Ie421f0cbfaf07fb87869c7965aed9579175f12db
Reviewed-on:
Reviewed-by: Natalie Chouinard <chouinard@google.com>
M DEPS
M third_party/dxc
ap...@google.com <ap...@google.com> #24
Branch: chromium/6367
commit e04b03f714994b7a747b5472da4ffae9e6e38938
Author: Antonio Maiorano <amaiorano@google.com>
Date: Mon May 27 20:28:40 2024
DEPS: Update DXC to patched branch
Bug: chromium:338103465
Change-Id: Ibc3677e42a81892c6f2f58bac7ee12409a7c6ae9
Reviewed-on:
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Natalie Chouinard <chouinard@google.com>
M DEPS
M third_party/dxc
sp...@google.com <sp...@google.com> #26
Hello,
Congratulations! The Chrome Vulnerability Rewards Program (VRP) Panel has decided to award you $10000.00 for this report.
Rationale for this decision:
memory corruption in the GPU process
Thank you for your efforts and helping us make Chrome more secure for all users!
Cheers,
Chrome VRP Panel Bot
P.S. Two other things we'd like to mention:
* Please do NOT publicly disclose details until a fix has been released to all our users. Early public disclosure may cancel the provisional reward. Also, please be considerate about disclosure when the bug affects a core library that may be used by other products. Please do NOT share this information with third parties who are not directly involved in fixing the bug. Doing so may cancel the provisional reward. Please be honest if you have already disclosed anything publicly or to third parties. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an eligible charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing.
Please contact security-vrp@chromium.org with any questions.
* If you are not already registered with Google as a supplier, p2p-vrp@google.com will reach out to you. If you have already registered, there is no need to repeat the process and you’ll automatically be paid soon. If you have any payment related questions or issues, please reach out to p2p-vrp@google.com.
am...@chromium.org <am...@chromium.org> #27
Congratulations on yet another one, wgslfuzz! Thanks your for your efforts in GPU fuzzing and reporting these issues to us -- great work!
pe...@google.com <pe...@google.com> #28
This bug has been closed for more than 14 weeks. Removing issue access restrictions.
Description
VULNERABILITY DETAILS
Chromium translates wgsl shaders via tint to an OS-specific shader format. On Windows, the OS-specific file format is hlsl, processed in dxcompiler.dll. When compiling chrome-generated hlsl files, this triggers an UAF inside the hlsl compiler.
VERSION
Chrome Version: Pre-compiled ASAN Chromium 126.0.6451.0 (Developer Build) (64-bit)
Operating System: Win11 Build 22631.2861
REPRODUCTION CASE
Attached is a .html file containing a WebGPU shader. Opening the html file (on windows with the D3D12 backend) crashes the GPU process. Note that the shader compiler only crashes if the DirectX back-end supports shader model 6.6. Inspecting the shader model is possible with a Microsoft tool called dxcapsviewer. The UI on my machine shows:
Reproducing the issue stand-alone on Linux also possible:
./tint standalone.wgsl -o standalone.hlsl
. I also attached standalone.hlsl, but you should get the very same file when compiling standalone.wgsl yourself./dxc-3.7 standalone.hlsl -T cs_6_6
. This should trigger an ASAN violation.Attached:
I have a bunch of more crashes but I don't want to spam the bug tracker. Do you prefer that I (1) report them all or (2) wait for the current dxcompiler issues to be fixed, check which crashes remain and report the ones still reproducing?