Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-136549

Race condition and crash when HTTP error received while QHttp2Stream is uploading

    XMLWordPrintable

Details

    • macOS, Windows
    • b0b9adf06 (dev), 6b4e11e63 (6.9), 1cd79cf42 (6.9.1)

    Description

      There's a race condition between how `QHttp2Stream` uploads the body of a POST request and the simultaneous handling of HTTP error responses. If certain events happen in an unfavourable order, Qt will crash in `QHttp2ProtocolHandler::finishStreamWithError()` with use-after-free. Here's the description of the events which lead to the crash:

      • A POST request is started with `QNetworkAccessManager::post`
      • `QHttp2Stream` starts an upload but the upload hasn't finished yet (`m_uploadByteDevice->atEnd()` is false, `finishSendDATA` is not yet called). 
      • The POST endpoint replies with an error while the request body is still uploading
      • The handling of the error causes the operation to be cancelled, `QHttpThreadDelegate`'s destructor is called
      • Within the destructor call, the owned upload device (`QNonContiguousByteDeviceThreadForwardImpl`) also gets destructed, whose `QObject::destroyed` signal notifies the slot `QHttp2Stream::uploadDeviceDestroyed` which emits `errorOccured` with the text "Upload device destroyed while uploading" which ends up calling `QHttp2ProtocolHandler::finishStreamWithError`
      • The crash happens in`QHttp2ProtocolHandler::finishStreamWithError` in the call to `httpReply->disconnect(this)`

      I created a minimal reproducible example by adding a few lines to the `qtbase/examples/network/http` example. 

       

      You can see the changes to example in this diff: https://github.com/tamaskenezlego/qtbase/compare/v6.9.0...qhttp2stream_race_condition or you can download the attached sources in `http_modified.zip`.

       

      How the modified example works:

      I replaced the user defined URL in the example with our endpoint which accepts a POST request but replies with a 401 after a while. 

      I also added heavy load to the user thread by scheduling 10ms sleeps on it. Because of the busy user thread the synchronization between the user thread and the http thread will be lagging and the server's 401 response has a great chance to arrive when the upload is still ongoing.

       

      In order to trigger the crash with the modified example, run the program, leave all the fields as they are and keep on clicking on the `Download` button. A couple of download attempts should trigger the crash.

      Attached is the stack trace after the crash (screenshot).

       

      Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

          People

            manordheim Mårten Nordheim
            tamas_kenez Tamas Kenez
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Gerrit Reviews

                There are no open Gerrit changes