Changeset 248014 in webkit


Ignore:
Timestamp:
Jul 30, 2019 12:54:27 PM (5 years ago)
Author:
Chris Dumez
Message:

Fix non-thread safe use of WeakPtr under sendSecItemRequest()
https://bugs.webkit.org/show_bug.cgi?id=200249

Reviewed by Alex Christensen.

The function was calling globalNetworkProcess() from a background thread. This is not safe because
globalNetworkProcess() deferences a WeakPtr<NetworkProcess> internally and the NetworkProcess object
gets destroyed on the main thread.

  • Shared/mac/SecItemShim.cpp:

(WebKit::sendSecItemRequest):

Location:
trunk/Source/WebKit
Files:
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebKit/ChangeLog

    r248010 r248014  
     12019-07-30  Chris Dumez  <cdumez@apple.com>
     2
     3        Fix non-thread safe use of WeakPtr under sendSecItemRequest()
     4        https://bugs.webkit.org/show_bug.cgi?id=200249
     5
     6        Reviewed by Alex Christensen.
     7
     8        The function was calling globalNetworkProcess() from a background thread. This is not safe because
     9        globalNetworkProcess() deferences a WeakPtr<NetworkProcess> internally and the NetworkProcess object
     10        gets destroyed on the main thread.
     11
     12        * Shared/mac/SecItemShim.cpp:
     13        (WebKit::sendSecItemRequest):
     14
    1152019-07-24  Carlos Garcia Campos  <cgarcia@igalia.com>
    216
  • trunk/Source/WebKit/Shared/mac/SecItemShim.cpp

    r248006 r248014  
    6565}
    6666
    67 static WorkQueue& workQueue()
    68 {
    69     static WorkQueue* workQueue;
    70     static dispatch_once_t onceToken;
    71     dispatch_once(&onceToken, ^{
    72         workQueue = &WorkQueue::create("com.apple.WebKit.SecItemShim").leakRef();
    73 
    74     });
    75 
    76     return *workQueue;
    77 }
    78 
    7967static Optional<SecItemResponseData> sendSecItemRequest(SecItemRequestData::Type requestType, CFDictionaryRef query, CFDictionaryRef attributesToMatch = 0)
    8068{
    81     if (!globalNetworkProcess())
    82         return WTF::nullopt;
     69    Optional<SecItemResponseData> response;
    8370
    84     Optional<SecItemResponseData> response;
     71    if (RunLoop::isMain()) {
     72        if (!globalNetworkProcess()->parentProcessConnection()->sendSync(Messages::SecItemShimProxy::SecItemRequestSync(SecItemRequestData(requestType, query, attributesToMatch)), Messages::SecItemShimProxy::SecItemRequestSync::Reply(response), 0))
     73            return WTF::nullopt;
     74        return response;
     75    }
    8576
    8677    BinarySemaphore semaphore;
    8778
    88     globalNetworkProcess()->parentProcessConnection()->sendWithReply(Messages::SecItemShimProxy::SecItemRequest(SecItemRequestData(requestType, query, attributesToMatch)), 0, workQueue(), [&response, &semaphore](auto reply) {
    89         if (reply)
    90             response = WTFMove(std::get<0>(*reply));
     79    RunLoop::main().dispatch([&] {
     80        if (!globalNetworkProcess()) {
     81            semaphore.signal();
     82            return;
     83        }
    9184
    92         semaphore.signal();
     85        globalNetworkProcess()->parentProcessConnection()->sendWithAsyncReply(Messages::SecItemShimProxy::SecItemRequest(SecItemRequestData(requestType, query, attributesToMatch)), [&](auto reply) {
     86            if (reply)
     87                response = WTFMove(*reply);
     88
     89            semaphore.signal();
     90        });
    9391    });
    9492
  • trunk/Source/WebKit/UIProcess/mac/SecItemShimProxy.cpp

    r248006 r248014  
    5757}
    5858
    59 void SecItemShimProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
    60 {
    61 }
    62 
    63 void SecItemShimProxy::secItemRequest(const SecItemRequestData& request, CompletionHandler<void(SecItemResponseData&&)>&& response)
     59void SecItemShimProxy::secItemRequest(const SecItemRequestData& request, CompletionHandler<void(Optional<SecItemResponseData>&&)>&& response)
    6460{
    6561    switch (request.type()) {
     
    9894}
    9995
     96void SecItemShimProxy::secItemRequestSync(const SecItemRequestData& data, CompletionHandler<void(Optional<SecItemResponseData>&&)>&& completionHandler)
     97{
     98    secItemRequest(data, WTFMove(completionHandler));
     99}
     100
    100101} // namespace WebKit
    101102
  • trunk/Source/WebKit/UIProcess/mac/SecItemShimProxy.h

    r248006 r248014  
    4949    void didReceiveSyncMessage(IPC::Connection&, IPC::Decoder&, std::unique_ptr<IPC::Encoder>&) override;
    5050
    51     void secItemRequest(const SecItemRequestData&, CompletionHandler<void(SecItemResponseData&&)>&&);
     51    void secItemRequest(const SecItemRequestData&, CompletionHandler<void(Optional<SecItemResponseData>&&)>&&);
     52    void secItemRequestSync(const SecItemRequestData&, CompletionHandler<void(Optional<SecItemResponseData>&&)>&&);
    5253
    5354    Ref<WorkQueue> m_queue;
  • trunk/Source/WebKit/UIProcess/mac/SecItemShimProxy.messages.in

    r248006 r248014  
    2424
    2525#if ENABLE(SEC_ITEM_SHIM)
    26     SecItemRequest(WebKit::SecItemRequestData request) -> (WebKit::SecItemResponseData response) Synchronous
     26    SecItemRequestSync(WebKit::SecItemRequestData request) -> (Optional<WebKit::SecItemResponseData> response) Synchronous
     27    SecItemRequest(WebKit::SecItemRequestData request) -> (Optional<WebKit::SecItemResponseData> response) Async
    2728#endif
    2829
Note: See TracChangeset for help on using the changeset viewer.