From 23f5a846f6702c456cf7cc9490e50cfd23368910 Mon Sep 17 00:00:00 2001

From: Max Reitz <mreitz@redhat.com>

Date: Tue, 19 Nov 2019 15:29:59 +0000

Subject: [PATCH 7/8] curl: Handle success in multi_check_completion

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

RH-Author: Max Reitz <mreitz@redhat.com>

Message-id: <20191119153000.101646-7-mreitz@redhat.com>

Patchwork-id: 92520

O-Subject: [RHEL-8.2.0 qemu-kvm PATCH 6/7] curl: Handle success in multi_check_completion

Bugzilla: 1744602

RH-Acked-by: Maxim Levitsky <mlevitsk@redhat.com>

RH-Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com>

RH-Acked-by: Stefano Garzarella <sgarzare@redhat.com>

Background: As of cURL 7.59.0, it verifies that several functions are

not called from within a callback.  Among these functions is

curl_multi_add_handle().

curl_read_cb() is a callback from cURL and not a coroutine.  Waking up

acb->co will lead to entering it then and there, which means the current

request will settle and the caller (if it runs in the same coroutine)

may then issue the next request.  In such a case, we will enter

curl_setup_preadv() effectively from within curl_read_cb().

Calling curl_multi_add_handle() will then fail and the new request will

not be processed.

Fix this by not letting curl_read_cb() wake up acb->co.  Instead, leave

the whole business of settling the AIOCB objects to

curl_multi_check_completion() (which is called from our timer callback

and our FD handler, so not from any cURL callbacks).

Reported-by: Natalie Gavrielov <ngavrilo@redhat.com>

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1740193

Cc: qemu-stable@nongnu.org

Signed-off-by: Max Reitz <mreitz@redhat.com>

Message-id: 20190910124136.10565-7-mreitz@redhat.com

Reviewed-by: John Snow <jsnow@redhat.com>

Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

(cherry picked from commit bfb23b480a49114315877aacf700b49453e0f9d9)

Signed-off-by: Max Reitz <mreitz@redhat.com>

Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>

---

 block/curl.c | 69 +++++++++++++++++++++++++-----------------------------------

 1 file changed, 29 insertions(+), 40 deletions(-)

diff --git a/block/curl.c b/block/curl.c

index f776615..b5899e1 100644

--- a/block/curl.c

+++ b/block/curl.c

@@ -238,7 +238,6 @@ static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void *opaque)

 {

     CURLState *s = ((CURLState*)opaque);

     size_t realsize = size * nmemb;

-    int i;

     DPRINTF("CURL: Just reading %zd bytes\n", realsize);

@@ -254,32 +253,6 @@ static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void *opaque)

     memcpy(s->orig_buf + s->buf_off, ptr, realsize);

     s->buf_off += realsize;

-    for(i=0; i

-        CURLAIOCB *acb = s->acb[i];

-

-        if (!acb)

-            continue;

-

-        if ((s->buf_off >= acb->end)) {

-            size_t request_length = acb->bytes;

-

-            qemu_iovec_from_buf(acb->qiov, 0, s->orig_buf + acb->start,

-                                acb->end - acb->start);

-

-            if (acb->end - acb->start < request_length) {

-                size_t offset = acb->end - acb->start;

-                qemu_iovec_memset(acb->qiov, offset, 0,

-                                  request_length - offset);

-            }

-

-            acb->ret = 0;

-            s->acb[i] = NULL;

-            qemu_mutex_unlock(&s->s->mutex);

-            aio_co_wake(acb->co);

-            qemu_mutex_lock(&s->s->mutex);

-        }

-    }

-

 read_end:

     /* curl will error out if we do not return this value */

     return size * nmemb;

@@ -360,13 +333,14 @@ static void curl_multi_check_completion(BDRVCURLState *s)

             break;

         if (msg->msg == CURLMSG_DONE) {

+            int i;

             CURLState *state = NULL;

+            bool error = msg->data.result != CURLE_OK;

+

             curl_easy_getinfo(msg->easy_handle, CURLINFO_PRIVATE,

                               (char **)&state);

-            /* ACBs for successful messages get completed in curl_read_cb */

-            if (msg->data.result != CURLE_OK) {

-                int i;

+            if (error) {

                 static int errcount = 100;

                 /* Don't lose the original error message from curl, since

@@ -378,20 +352,35 @@ static void curl_multi_check_completion(BDRVCURLState *s)

                         error_report("curl: further errors suppressed");

                     }

                 }

+            }

-                for (i = 0; i < CURL_NUM_ACB; i++) {

-                    CURLAIOCB *acb = state->acb[i];

+            for (i = 0; i < CURL_NUM_ACB; i++) {

+                CURLAIOCB *acb = state->acb[i];

-                    if (acb == NULL) {

-                        continue;

-                    }

+                if (acb == NULL) {

+                    continue;

+                }

+

+                if (!error) {

+                    /* Assert that we have read all data */

+                    assert(state->buf_off >= acb->end);

+

+                    qemu_iovec_from_buf(acb->qiov, 0,

+                                        state->orig_buf + acb->start,

+                                        acb->end - acb->start);

-                    acb->ret = -EIO;

-                    state->acb[i] = NULL;

-                    qemu_mutex_unlock(&s->mutex);

-                    aio_co_wake(acb->co);

-                    qemu_mutex_lock(&s->mutex);

+                    if (acb->end - acb->start < acb->bytes) {

+                        size_t offset = acb->end - acb->start;

+                        qemu_iovec_memset(acb->qiov, offset, 0,

+                                          acb->bytes - offset);

+                    }

                 }

+

+                acb->ret = error ? -EIO : 0;

+                state->acb[i] = NULL;

+                qemu_mutex_unlock(&s->mutex);

+                aio_co_wake(acb->co);

+                qemu_mutex_lock(&s->mutex);

             }

             curl_clean_state(state);

-- 

1.8.3.1