[webkit-changes] [WebKit/WebKit] 0d7840: Potential unsigned integer underflow in JSC::Funct...
Dan Hecht
noreply at github.com
Wed Dec 18 10:35:59 PST 2024
Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 0d784010f50ea442e32b2fb0edd4b1d113bee66e
https://github.com/WebKit/WebKit/commit/0d784010f50ea442e32b2fb0edd4b1d113bee66e
Author: David Kilzer <ddkilzer at apple.com>
Date: 2024-12-18 (Wed, 18 Dec 2024)
Changed paths:
M Source/JavaScriptCore/tools/FunctionAllowlist.cpp
Log Message:
-----------
Potential unsigned integer underflow in JSC::FunctionAllowlist::FunctionAllowlist constructor
<https://bugs.webkit.org/show_bug.cgi?id=281675>
<rdar://138127490>
Reviewed by Darin Adler.
* Source/JavaScriptCore/tools/FunctionAllowlist.cpp:
(JSC::FunctionAllowlist::FunctionAllowlist):
- Check that `length` returned from strlen() is non-zero before checking
the end of the C-string for a newline character.
Originally-landed-as: 283286.315 at safari-7620-branch (98b6fa893826). rdar://141318696
Canonical link: https://commits.webkit.org/288019@main
Commit: 1b27a467e7957fc4c49fb294d8f244d00127c2c2
https://github.com/WebKit/WebKit/commit/1b27a467e7957fc4c49fb294d8f244d00127c2c2
Author: Kimmo Kinnunen <kkinnunen at apple.com>
Date: 2024-12-18 (Wed, 18 Dec 2024)
Changed paths:
M Source/ThirdParty/ANGLE/src/compiler/translator/msl/EmitMetal.cpp
M Source/ThirdParty/ANGLE/src/tests/angle_end2end_tests.gni
A Source/ThirdParty/ANGLE/src/tests/gl_tests/TimeoutDrawTest.cpp
Log Message:
-----------
WebGL infinite loops are optimized out in certain cases
https://bugs.webkit.org/show_bug.cgi?id=281902
rdar://136486349
Reviewed by Mike Wyrzykowski.
Metal: Ensure potentially infinite loops have defined behavior
The MSL compiler would omit infinite loops and assume number domains
based on the omission logic. This would induce incorrect number domains
in case the infinite loops would be invokable. Infinite loops are
undefined in C++ and thus in MSL. It is the job of the programmer
to ensure undefined behavior cannot happen.
Consider GLSL loop like:
uniform float i;
...
if (i != 0.5) for(;;) { }
gl_FragColor = vec4(i);
Historically this would emit MSL loop in spirit of:
if (i != 0.5) {
bool c = true;
while (c) { }
}
ANGLE_fragmentOut.gl_FragColor = metal::float4(i, i, i, i);
Since This could cause the MSL compiler to optimize the function to
equivalent of:
ANGLE_fragmentOut.gl_FragColor = metal::float4(0.5, 0.5, 0.5, 0.5);
Presumably this loop omission would happen at the clang frontend part.
Before, was worked around by emitting asm statements to the MSL:
bool c = true;
while (c) {
__asm__("");
}
The asm injection would would work for this particular source pattern,
presumably because injecting the asm would avoid the loop omission at
the clang frontend part.
The MSL/C++ code is still UB, though. The asm statement does not cause
anything that C++ would consider as "forward progress" of the loop. The
success was just due to how the backend worked. The bitcode produced
would be similar to:
4:
tail call void asm sideeffect "", ""() #6, !srcloc !28
br label %4, !llvm.loop !29
Here, the compiler can be seen to simply fail to detect a loop that
does not make forward progress.
Considering GLSL of form:
uniform int f;
...
for (;;) { if (f <= 1) break; }
With asm injection to the loop, this would produce:
5:
tail call void asm sideeffect "", ""() #8, !srcloc !29
%6 = load i32, i32 addrspace(2)* %4, align 4, !tbaa !30
%7 = icmp slt i32 %6, 2
br i1 %7, label %8, label %5
8:
This code is still assumed to make progress. The backend optimizer is
free to assume that the condition holds, since the load to break the
loop is from constant address space. I.e. uniform f does not change its
value during the loop.
Instead of injecting asm, inject a read of unused volatile variable.
The volatile variable access is defined in C++ as forward progress.
This means infinite loop containing such read is considered defined.
To simplify the implementation and to avoid volatile writes, the read
is to a dummy variable instead of the loop condition bool.
The tests here do not pass completely for MSL backend. In case the
compiler would omit the infinite loop (unpatched code), they would fail
with demonstration of how the values behave. After fixing, the loops
cause timeout but Metal backend does not have implementation to report
context loss. Also, the ReadPixels is just for demostration purposes of the
unpatched code.
* Source/ThirdParty/ANGLE/src/compiler/translator/msl/EmitMetal.cpp:
(GenMetalTraverser::GenMetalTraverser):
(GenMetalTraverser::emitLoopBody):
(GenMetalTraverser::emitForwardProgressStore):
(GenMetalTraverser::emitForwardProgressSignal):
(GenMetalTraverser::visitForLoop):
(GenMetalTraverser::visitWhileLoop):
(GenMetalTraverser::visitDoWhileLoop):
* Source/ThirdParty/ANGLE/src/tests/angle_end2end_tests.gni:
* Source/ThirdParty/ANGLE/src/tests/gl_tests/TimeoutDrawTest.cpp: Added.
(angle::TimeoutDrawTest::TimeoutDrawTest):
(angle::TEST_P):
Originally-landed-as: 283286.350 at safari-7620-branch (b82d94e60f49). rdar://141318430
Canonical link: https://commits.webkit.org/288020@main
Commit: efb106b8c785760b692c24a000dbbb0ca2cfea47
https://github.com/WebKit/WebKit/commit/efb106b8c785760b692c24a000dbbb0ca2cfea47
Author: Nitin Mahendru <nitinmahendru at apple.com>
Date: 2024-12-18 (Wed, 18 Dec 2024)
Changed paths:
A LayoutTests/security/contentSecurityPolicy/block-javascripturl-non-inline-csp-expected.txt
A LayoutTests/security/contentSecurityPolicy/block-javascripturl-non-inline-csp.html
A LayoutTests/security/contentSecurityPolicy/resources/csp-javascript-url.js
A LayoutTests/security/contentSecurityPolicy/resources/no-csp.html
M Source/WebCore/loader/FrameLoader.cpp
Log Message:
-----------
`javascript: url` navigation to another browsing context (created from `window.open`) misses checking the source's CSP
rdar://137941234
https://bugs.webkit.org/show_bug.cgi?id=281477
Reviewed by Alex Christensen.
A change in window.location.href causes a navigation. Were were not checking the CSP in
that flow. This change adds that.
* LayoutTests/security/contentSecurityPolicy/block-javascripturl-non-inline-csp-expected.txt: Added.
* LayoutTests/security/contentSecurityPolicy/block-javascripturl-non-inline-csp.html: Added.
* LayoutTests/security/contentSecurityPolicy/resources/csp-javascript-url.js: Added.
(sleep):
(sleep.500.then):
* LayoutTests/security/contentSecurityPolicy/resources/no-csp.html: Added.
* Source/WebCore/loader/FrameLoader.cpp:
(WebCore::FrameLoader::loadWithNavigationAction):
Originally-landed-as: 283286.352 at safari-7620-branch (378ba1584ade). rdar://141318344
Canonical link: https://commits.webkit.org/288021@main
Commit: 477e22399befde2a767c6d0c3742f0c00f1169f5
https://github.com/WebKit/WebKit/commit/477e22399befde2a767c6d0c3742f0c00f1169f5
Author: Aditya Keerthi <akeerthi at apple.com>
Date: 2024-12-18 (Wed, 18 Dec 2024)
Changed paths:
M Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.h
M Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm
M Tools/TestWebKitAPI/Tests/mac/ContextMenuTests.mm
Log Message:
-----------
[CoreIPC] Messages::WebPageProxy::ShowContextMenuFromFrame allows arbitrary PDF rendering in UI process from web content
https://bugs.webkit.org/show_bug.cgi?id=282103
rdar://135720091
Reviewed by Wenson Hsieh.
WebKit supplies images to the sharing service picker so that they can be shared
out to the system when the user right clicks on an image and clicks "Share".
Since the sharing service picker must be created in order to obtain the menu item
itself, the supplied `NSImage` is created when the context menu is displayed, rather
than when the user clicks "Share". This means that the code which creates the
`NSImage` is reachable simply with the `ShowContextMenuFromFrame` IPC message.
Additionally, the image is created with raw data rather than using a ShareableBitmap,
as the original format should be preserved when sharing. Consequently, the IPC
message can act as a way to perform image processing in the UI process.
To fix, supply a placeholder `NSImage` when creating the sharing service picker,
solely for the purpose of obtaining the menu item. Then, when the menu item is
clicked, another item is created with the real image, and then programmatically
invoked to present the sharing picker. This solution ensures that image processing
does not occur until the user actually clicks "Share".
* Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.h:
* Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm:
(-[WKMenuTarget performShare:]):
(WebKit::WebContextMenuProxyMac::handleShareMenuItem):
Set a `menu` on the created menu item, to ensure that the sharing service
picker has knowledge of the presenting view and the associated context menu.
The picker uses that information to make decisions about its presentation.
(WebKit::WebContextMenuProxyMac::createShareMenuItem):
Specify a placeholder image when needed, and simply grab the title of the sharing
menu item to create an item for display in the context menu.
(WebKit::WebContextMenuProxyMac::getContextMenuItem):
(WebKit::WebContextMenuProxyMac::getShareMenuItem): Deleted.
* Tools/TestWebKitAPI/Tests/mac/ContextMenuTests.mm:
(-[NSSharingServicePicker swizzled_initWithItems:]):
(TestWebKitAPI::TEST(ContextMenuTests, ShowSharePopoverForImage)):
Add a test to ensure that the sharing picker is still presented when right clicking
on an image and clicking share, and that the image is preserved.
Originally-landed-as: 283286.355 at safari-7620-branch (c2423adca8d6). rdar://141318263
Canonical link: https://commits.webkit.org/288022@main
Commit: 032dd069dcf03d18e0b2f21b0955f55d09e1ab83
https://github.com/WebKit/WebKit/commit/032dd069dcf03d18e0b2f21b0955f55d09e1ab83
Author: Dan Hecht <dan.hecht at apple.com>
Date: 2024-12-18 (Wed, 18 Dec 2024)
Changed paths:
A JSTests/wasm/stress/compile-unreachable-catch.js
M Source/JavaScriptCore/wasm/WasmBBQJIT.cpp
M Source/JavaScriptCore/wasm/WasmBBQJIT.h
Log Message:
-----------
[JSC] BBQJIT::addCatchToUnreachable should unbind all temps
https://bugs.webkit.org/show_bug.cgi?id=282180
rdar://138178927
Reviewed by David Degazio.
BBQJIT::addCatchToUnreachable() and BBQJIT::addCatchAllToUnreachable()
are used after a control flow instruction is reached that makes the
end of the block unreachable, so they both avoid emitting code to
flush temps. However, they still need to unbind temps, otherwise
temps that are used within the catch will refer to stale bindings.
This issue occurs when catch or catch_all follows return_call or
unreachable bytecodes as these do not themselves flush the temps
back to their canonical locations (whereas uncondtional branch will since
the temps can still be live).
* JSTests/wasm/stress/compile-unreachable-catch.js: Added.
(async runOne):
* Source/JavaScriptCore/wasm/WasmBBQJIT.cpp:
(JSC::Wasm::BBQJITImpl::BBQJIT::addCatchToUnreachable):
(JSC::Wasm::BBQJITImpl::BBQJIT::addCatchAllToUnreachable):
(JSC::Wasm::BBQJITImpl::BBQJIT::addEndToUnreachable):
(JSC::Wasm::BBQJITImpl::BBQJIT::unbindAllRegisters):
* Source/JavaScriptCore/wasm/WasmBBQJIT.h:
Originally-landed-as: 283286.367 at safari-7620-branch (6146215d9220). rdar://141318083
Canonical link: https://commits.webkit.org/288023@main
Compare: https://github.com/WebKit/WebKit/compare/8a63c8a4db4f...032dd069dcf0
To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications
More information about the webkit-changes
mailing list