Skip to content
Snippets Groups Projects
  • Hanna Reitz's avatar
    0f418a20
    curl: Disconnect sockets from CURLState · 0f418a20
    Hanna Reitz authored
    When a curl transfer is finished, that does not mean that CURL lets go
    of all the sockets it used for it.  We therefore must not free a
    CURLSocket object before CURL has invoked curl_sock_cb() to tell us to
    remove it.  Otherwise, we may get a use-after-free, as described in this
    bug report: https://bugs.launchpad.net/qemu/+bug/1916501
    
    (Reproducer from that report:
      $ qemu-img convert -f qcow2 -O raw \
      https://download.cirros-cloud.net/0.4.0/cirros-0.4.0-x86_64-disk.img \
      out.img
    )
    
    (Alternatively, it might seem logical to force-drop all sockets that
    have been used for a state when the respective transfer is done, kind of
    like it is done now, but including unsetting the AIO handlers.
    Unfortunately, doing so makes the driver just hang instead of crashing,
    which seems to evidence that CURL still uses those sockets.)
    
    Make the CURLSocket object independent of "its" CURLState by putting all
    sockets into a hash table belonging to the BDRVCURLState instead of a
    list that belongs to a CURLState.  Do not touch any sockets in
    curl_clean_state().
    
    Testing, it seems like all sockets are indeed gone by the time the curl
    BDS is closed, so it seems like there really was no point in freeing any
    socket just because a transfer is done.  libcurl does invoke
    curl_sock_cb() with CURL_POLL_REMOVE for every socket it has.
    
    Buglink: https://bugs.launchpad.net/qemu/+bug/1916501
    
    
    Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
    Message-Id: <20210309130541.37540-3-mreitz@redhat.com>
    Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
    0f418a20
    History
    curl: Disconnect sockets from CURLState
    Hanna Reitz authored
    When a curl transfer is finished, that does not mean that CURL lets go
    of all the sockets it used for it.  We therefore must not free a
    CURLSocket object before CURL has invoked curl_sock_cb() to tell us to
    remove it.  Otherwise, we may get a use-after-free, as described in this
    bug report: https://bugs.launchpad.net/qemu/+bug/1916501
    
    (Reproducer from that report:
      $ qemu-img convert -f qcow2 -O raw \
      https://download.cirros-cloud.net/0.4.0/cirros-0.4.0-x86_64-disk.img \
      out.img
    )
    
    (Alternatively, it might seem logical to force-drop all sockets that
    have been used for a state when the respective transfer is done, kind of
    like it is done now, but including unsetting the AIO handlers.
    Unfortunately, doing so makes the driver just hang instead of crashing,
    which seems to evidence that CURL still uses those sockets.)
    
    Make the CURLSocket object independent of "its" CURLState by putting all
    sockets into a hash table belonging to the BDRVCURLState instead of a
    list that belongs to a CURLState.  Do not touch any sockets in
    curl_clean_state().
    
    Testing, it seems like all sockets are indeed gone by the time the curl
    BDS is closed, so it seems like there really was no point in freeing any
    socket just because a transfer is done.  libcurl does invoke
    curl_sock_cb() with CURL_POLL_REMOVE for every socket it has.
    
    Buglink: https://bugs.launchpad.net/qemu/+bug/1916501
    
    
    Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
    Message-Id: <20210309130541.37540-3-mreitz@redhat.com>
    Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>