[webkit-changes] [WebKit/WebKit] 5bcdab: Cherry-pick 252432.1029 at safari-7614-branch (9dda7b...

Ryan Reno noreply at github.com
Tue Mar 28 15:50:20 PDT 2023


  Branch: refs/heads/webkitglib/2.40
  Home:   https://github.com/WebKit/WebKit
  Commit: 5bcdabb358df7868634b33d3d48f6a270e80c157
      https://github.com/WebKit/WebKit/commit/5bcdabb358df7868634b33d3d48f6a270e80c157
  Author: Yijia Huang <yijia_huang at apple.com>
  Date:   2023-03-29 (Wed, 29 Mar 2023)

  Changed paths:
    A JSTests/wasm/stress/wasm-tuple-return.js
    M Source/JavaScriptCore/wasm/WasmLLIntGenerator.cpp

  Log Message:
  -----------
  Cherry-pick 252432.1029 at safari-7614-branch (9dda7bfe768d). https://bugs.webkit.org/show_bug.cgi?id=250482

    LLInt WASM argument locals must be read before return values are written
    https://bugs.webkit.org/show_bug.cgi?id=250482
    rdar://103551585

    Reviewed by Justin Michaud.

    Given the wasm code which exports a wasm function `intFuncRef2` as a js function.
    ```
    (func (export "intFuncRef2") (param $p0 f32) (param $p1 funcref) (result i32 funcref)
        (i32.const 42)
        (local.get $p1)
        (return)
    )
    ```
    The corresponding dumped bytecodes show
    ```
    [   0] enter
    [   1] mov     dst:loc2, src:42(const0)
    [   4] mov     dst:loc3, src:loc2       // loc2 contains the funcref but now replaced with 42
    [   7] ret                              // return [loc2, loc3]
    ```
    which is wrong. Instead we should do
    ```
    [   0] enter
    [   1] mov     dst:loc18, src:42(const0)
    [   4] mov     dst:loc19, src:loc2
    [   7] mov     dst:loc2, src:loc18
    [  10] mov     dst:loc3, src:loc19
    [  13] ret
    ```
    Note that loc2 is both parameter and return lot.

    Locals usually need to be materialized on wasm stack when they are about to be or could
    be clobbered, usually before a control entry, a branch, or redefinition. Previously,
    Return writes only one value to the result slot that clobber one argument slot which
    is fine. Since now wasm function can return tuple that might bring us to the situation
    as shown in above example. We should materialize expression stack when return more than
    one values.

    * JSTests/wasm/stress/tuple-return.js: Added.
    (async test):
    * Source/JavaScriptCore/wasm/WasmLLIntGenerator.cpp:
    (JSC::Wasm::LLIntGenerator::addReturn):

    Canonical link: https://commits.webkit.org/252432.1029@safari-7614-branch


  Commit: 5bad5db3251c892ca60ff6f06d28479203d63a57
      https://github.com/WebKit/WebKit/commit/5bad5db3251c892ca60ff6f06d28479203d63a57
  Author: Chris Dumez <cdumez at apple.com>
  Date:   2023-03-29 (Wed, 29 Mar 2023)

  Changed paths:
    M Source/WebCore/bindings/js/JSErrorHandler.cpp
    M Source/WebCore/bindings/js/JSEventListener.cpp
    M Source/WebCore/bindings/js/JSEventListener.h
    M Source/WebCore/bindings/js/JSLazyEventListener.cpp
    M Source/WebCore/bindings/js/WebCoreJSClientData.cpp
    M Source/WebCore/bindings/js/WebCoreJSClientData.h
    M Source/WebCore/dom/EventTarget.cpp
    M Source/WebCore/inspector/CommandLineAPIHost.cpp
    M Source/WebCore/inspector/WebInjectedScriptHost.cpp
    M Source/WebCore/inspector/agents/InspectorDOMAgent.cpp

  Log Message:
  -----------
  Cherry-pick 252432.1030 at safari-7614-branch (433db4f29219). https://bugs.webkit.org/show_bug.cgi?id=246022

    Heap use-after-free in DOMWrapperWorld::~DOMWrapperWorld
    https://bugs.webkit.org/show_bug.cgi?id=246022
    rdar://100763856

    Reviewed by Jonathan Bedard and Ryosuke Niwa.

    Right before a worker terminates, it destroys its WorkerOrWorkletScriptController,
    which destroys the JS VM. Certain objects like DOMWrapperWorld cannot outlive
    the VM since they keep a `VM&' as data member. However, DOMWrapperWorld is
    refcounted and JSEventListeners hold a strong ref to their DOMWrapperWorld. If
    JSEventListeners outlive the VM, then it would lead to a use-after free in the
    DOMWrapperWorld destructor when destroying those JSEventListeners later on.

    We have previously made several attempts to try and unregister all event
    listeners before destroying the VM. However, those attempts were either
    incomplete or led to other crashes. I am therefore trying a different approach
    this time.

    JSEventListeners now register themselves as client of the JSVMClientData (which
    is owned by the VM) and the client gets a `willDestroyVM()` call before the
    VM gets destroyed. This allows JSEventListeners to clear out their data members
    which rely on the VM (DOMWrapperWorld and JSC::Weak data members).

    * Source/WebCore/bindings/js/JSErrorHandler.cpp:
    (WebCore::JSErrorHandler::handleEvent):
    * Source/WebCore/bindings/js/JSEventListener.cpp:
    (WebCore::JSEventListener::JSEventListener):
    (WebCore::JSEventListener::handleEvent):
    (WebCore::JSEventListener::functionName const):
    (WebCore::JSEventListener::willDestroyVM):
    * Source/WebCore/bindings/js/JSEventListener.h:
    (WebCore::JSEventListener::isolatedWorld const):
    (WebCore::JSEventListener::ensureJSFunction const):
    * Source/WebCore/bindings/js/JSLazyEventListener.cpp:
    (WebCore::JSLazyEventListener::initializeJSFunction const):
    * Source/WebCore/bindings/js/WebCoreJSClientData.cpp:
    (WebCore::JSVMClientData::~JSVMClientData):
    * Source/WebCore/bindings/js/WebCoreJSClientData.h:
    (WebCore::JSVMClientData::addClient):
    * Source/WebCore/dom/EventTarget.cpp:
    (WebCore::EventTarget::attributeEventListener):
    * Source/WebCore/inspector/CommandLineAPIHost.cpp:
    (WebCore::CommandLineAPIHost::getEventListeners):
    * Source/WebCore/inspector/WebInjectedScriptHost.cpp:
    (WebCore::objectForEventTargetListeners):
    * Source/WebCore/inspector/agents/InspectorDOMAgent.cpp:
    (WebCore::InspectorDOMAgent::buildObjectForEventListener):

    Canonical link: https://commits.webkit.org/252432.1030@safari-7614-branch


  Commit: 42498db72eabcb51b4f706c6da5b667fe517c943
      https://github.com/WebKit/WebKit/commit/42498db72eabcb51b4f706c6da5b667fe517c943
  Author: David Degazio <35146201+ddegazio at users.noreply.github.com>
  Date:   2023-03-29 (Wed, 29 Mar 2023)

  Changed paths:
    A JSTests/stress/cell-speculated-array-indexof.js
    M Source/JavaScriptCore/dfg/DFGFixupPhase.cpp

  Log Message:
  -----------
  Cherry-pick 252432.1031 at safari-7614-branch (9f7e401c42a8). https://bugs.webkit.org/show_bug.cgi?id=250429

    Fix use-after-free in DFGFixupPhase for array indexOf
    https://bugs.webkit.org/show_bug.cgi?id=250429
    rdar://103852510

    Reviewed by Jonathan Bedard and Michael Saboff.

    During DFG fixup, array indexOf nodes are folded to -1 when the search element is speculated
    to be a different type than the array element (for instance, JSCell instead of Int32). When
    this happens, a speculation check is inserted, which can cause the DFG graph's varArgChildren
    array to reallocate. This invalidates the searchElement Edge reference, which we use
    immediately after the check insertion in the fixup phase. This patch fixes this potential
    use-after-free by grabbing the searchElement's associated node before inserting any checks,
    giving us a persistent pointer to a DFG node rather than a reference into a vector.

    * JSTests/stress/cell-speculated-array-indexof.js: Added.
    * Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:
    (JSC::DFG::FixupPhase::fixupArrayIndexOf):

    Canonical link: https://commits.webkit.org/252432.1031@safari-7614-branch


  Commit: a4cc9fef4ed925d82d7e5af3b0d0c0e7d7aea67c
      https://github.com/WebKit/WebKit/commit/a4cc9fef4ed925d82d7e5af3b0d0c0e7d7aea67c
  Author: Simon Fraser <simon.fraser at apple.com>
  Date:   2023-03-29 (Wed, 29 Mar 2023)

  Changed paths:
    M Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp

  Log Message:
  -----------
  Cherry-pick 252432.1033 at safari-7614-branch (02e324c57689). https://bugs.webkit.org/show_bug.cgi?id=250742

    Possible type confusion bug in RemoteScrollingCoordinatorTransaction::decode
    https://bugs.webkit.org/show_bug.cgi?id=250742
    <rdar://102373218>

    Reviewed by Jonathan Bedard and Ryosuke Niwa.

    RemoteScrollingCoordinatorTransaction::decode() fails to check whether the nodeID returned by
    `m_scrollingStateTree->insertNode()` is a new one, different from the `nodeID` argument. If so, it
    could indicate that the node type of `m_scrollingStateTree->stateNodeForID()` does not match
    `nodeType`, leading to type confusion.

    In the UI process, `m_scrollingStateTree->insertNode()` should never return a different nodeID; this
    only happens when the given nodeType does not match the type of the existing node, which only
    happens in the WebProcess. So if `insertNode()` returns a different nodeID, or when the returned
    node doesn't have the expected type, we can consider it an IPC decoding error.

    * Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp:
    (WebKit::RemoteScrollingCoordinatorTransaction::decode):

    Canonical link: https://commits.webkit.org/252432.1033@safari-7614-branch


  Commit: 534a3a6ea4c438cab97a52ed16d0c61f763d0377
      https://github.com/WebKit/WebKit/commit/534a3a6ea4c438cab97a52ed16d0c61f763d0377
  Author: Ryan Reno <rreno at apple.com>
  Date:   2023-03-29 (Wed, 29 Mar 2023)

  Changed paths:
    A LayoutTests/imported/w3c/web-platform-tests/content-security-policy/generic/wildcard-host-checks-path.sub-expected.txt
    A LayoutTests/imported/w3c/web-platform-tests/content-security-policy/generic/wildcard-host-checks-path.sub.html
    M Source/WebCore/page/csp/ContentSecurityPolicySource.cpp

  Log Message:
  -----------
  Cherry-pick 252432.1034 at safari-7614-branch (3ee4a8321986). https://bugs.webkit.org/show_bug.cgi?id=250709

    CSP bypass due to incorrect handling of wildcard character in host expression
    https://bugs.webkit.org/show_bug.cgi?id=250709
    rdar://104335301

    Reviewed by Brent Fulgham and Jonathan Bedard.

    We were treating something like "https://*/foo" as being a scheme-only source (so checking only against
    'https'). That is fixed by not only checking for the host-part being an empty string but also whether or not
    the host wildcard flag had been set by the CSP parser. Additionally, we were checking a given URL's host
    against the wildcard assuming a format like "*.com" instead of the possibility of the catch-all "*" wildcard.

    This change fixes our handling of the wildcard "*" in a directive's source list by correctly identifying when a
    source is scheme-only and by correctly taking into account the entire host-part wildcard grammar when checking
    a given host against a wildcard pattern.

    * LayoutTests/imported/w3c/web-platform-tests/content-security-policy/generic/wildcard-host-checks-path.sub-expected.txt: Added.
    * LayoutTests/imported/w3c/web-platform-tests/content-security-policy/generic/wildcard-host-checks-path.sub.html: Added.
    * Source/WebCore/page/csp/ContentSecurityPolicySource.cpp:
    (WebCore::ContentSecurityPolicySource::hostMatches const):
    (WebCore::ContentSecurityPolicySource::isSchemeOnly const):

    Canonical link: https://commits.webkit.org/252432.1034@safari-7614-branch


Compare: https://github.com/WebKit/WebKit/compare/4524c8a9e1d4...534a3a6ea4c4


More information about the webkit-changes mailing list