Re: [PATCH] block/curl.c: Check error return from curl_easy_setopt()

["Daniel P. Berrangé" <berrange@redhat.com>]

On Fri, Jan 28, 2022 at 04:55:35PM +0000, Peter Maydell wrote:
> Coverity points out that we aren't checking the return value
> from curl_easy_setopt() for any of the calls to it we make
> in block/curl.c.
> 
> Fixes: Coverity CID 1459336, 1459482, 1460331
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Big fat disclaimer: tested only with 'make check', which I suspect
> may not be exercising this block backend. Hints on how to test
> more thoroughly are welcome.

yeah afaik, qemu-iotests don't have support for this. As a very
basic smoke test do

$ qemu-img info https://cloud.debian.org/images/cloud/buster/daily/latest/debian-10-nocloud-amd64-daily.qcow2
image: https://cloud.debian.org/images/cloud/buster/daily/latest/debian-10-nocloud-amd64-daily.qcow2
file format: qcow2
virtual size: 2 GiB (2147483648 bytes)
disk size: unavailable
cluster_size: 65536
Format specific information:
    compat: 1.1
    compression type: zlib
    lazy refcounts: false
    refcount bits: 16
    corrupt: false
    extended l2: false

and/or

$ qemu-img info --image-opts driver=qcow2,file.driver=https,file.url=https://cloud.debian.org/images/cloud/buster/daily/latest/debian-10-nocloud-amd64-daily.qcow2
image: https://cloud.debian.org/images/cloud/buster/daily/latest/debian-10-nocloud-amd64-daily.qcow2
file format: qcow2
virtual size: 2 GiB (2147483648 bytes)
disk size: unavailable
cluster_size: 65536
Format specific information:
    compat: 1.1
    compression type: zlib
    lazy refcounts: false
    refcount bits: 16
    corrupt: false
    extended l2: false


> 
>  block/curl.c | 90 +++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 58 insertions(+), 32 deletions(-)
> 
> diff --git a/block/curl.c b/block/curl.c
> index 6a6cd729758..aaee1b17bef 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -458,38 +458,51 @@ static int curl_init_state(BDRVCURLState *s, CURLState *state)
>          if (!state->curl) {
>              return -EIO;
>          }
> -        curl_easy_setopt(state->curl, CURLOPT_URL, s->url);
> -        curl_easy_setopt(state->curl, CURLOPT_SSL_VERIFYPEER,
> -                         (long) s->sslverify);
> -        curl_easy_setopt(state->curl, CURLOPT_SSL_VERIFYHOST,
> -                         s->sslverify ? 2L : 0L);
> -        if (s->cookie) {
> -            curl_easy_setopt(state->curl, CURLOPT_COOKIE, s->cookie);
> +        if (curl_easy_setopt(state->curl, CURLOPT_URL, s->url) ||
> +            curl_easy_setopt(state->curl, CURLOPT_SSL_VERIFYPEER,
> +                             (long) s->sslverify) ||
> +            curl_easy_setopt(state->curl, CURLOPT_SSL_VERIFYHOST,
> +                             s->sslverify ? 2L : 0L)) {
> +            goto err;
> +        }
> +        if (s->cookie) {
> +            if (curl_easy_setopt(state->curl, CURLOPT_COOKIE, s->cookie)) {
> +                goto err;
> +            }
> +        }
> +        if (curl_easy_setopt(state->curl, CURLOPT_TIMEOUT, (long)s->timeout) ||
> +            curl_easy_setopt(state->curl, CURLOPT_WRITEFUNCTION,
> +                             (void *)curl_read_cb) ||
> +            curl_easy_setopt(state->curl, CURLOPT_WRITEDATA, (void *)state) ||
> +            curl_easy_setopt(state->curl, CURLOPT_PRIVATE, (void *)state) ||
> +            curl_easy_setopt(state->curl, CURLOPT_AUTOREFERER, 1) ||
> +            curl_easy_setopt(state->curl, CURLOPT_FOLLOWLOCATION, 1) ||
> +            curl_easy_setopt(state->curl, CURLOPT_NOSIGNAL, 1) ||
> +            curl_easy_setopt(state->curl, CURLOPT_ERRORBUFFER, state->errmsg) ||
> +            curl_easy_setopt(state->curl, CURLOPT_FAILONERROR, 1)) {
> +            goto err;
>          }
> -        curl_easy_setopt(state->curl, CURLOPT_TIMEOUT, (long)s->timeout);
> -        curl_easy_setopt(state->curl, CURLOPT_WRITEFUNCTION,
> -                         (void *)curl_read_cb);
> -        curl_easy_setopt(state->curl, CURLOPT_WRITEDATA, (void *)state);
> -        curl_easy_setopt(state->curl, CURLOPT_PRIVATE, (void *)state);
> -        curl_easy_setopt(state->curl, CURLOPT_AUTOREFERER, 1);
> -        curl_easy_setopt(state->curl, CURLOPT_FOLLOWLOCATION, 1);
> -        curl_easy_setopt(state->curl, CURLOPT_NOSIGNAL, 1);
> -        curl_easy_setopt(state->curl, CURLOPT_ERRORBUFFER, state->errmsg);
> -        curl_easy_setopt(state->curl, CURLOPT_FAILONERROR, 1);
> -
>          if (s->username) {
> -            curl_easy_setopt(state->curl, CURLOPT_USERNAME, s->username);
> +            if (curl_easy_setopt(state->curl, CURLOPT_USERNAME, s->username)) {
> +                goto err;
> +            }
>          }
>          if (s->password) {
> -            curl_easy_setopt(state->curl, CURLOPT_PASSWORD, s->password);
> +            if (curl_easy_setopt(state->curl, CURLOPT_PASSWORD, s->password)) {
> +                goto err;
> +            }
>          }
>          if (s->proxyusername) {
> -            curl_easy_setopt(state->curl,
> -                             CURLOPT_PROXYUSERNAME, s->proxyusername);
> +            if (curl_easy_setopt(state->curl,
> +                                 CURLOPT_PROXYUSERNAME, s->proxyusername)) {
> +                goto err;
> +            }
>          }
>          if (s->proxypassword) {
> -            curl_easy_setopt(state->curl,
> -                             CURLOPT_PROXYPASSWORD, s->proxypassword);
> +            if (curl_easy_setopt(state->curl,
> +                                 CURLOPT_PROXYPASSWORD, s->proxypassword)) {
> +                goto err;
> +            }
>          }
>  
>          /* Restrict supported protocols to avoid security issues in the more
> @@ -499,18 +512,27 @@ static int curl_init_state(BDRVCURLState *s, CURLState *state)
>           * Restricting protocols is only supported from 7.19.4 upwards.
>           */
>  #if LIBCURL_VERSION_NUM >= 0x071304
> -        curl_easy_setopt(state->curl, CURLOPT_PROTOCOLS, PROTOCOLS);
> -        curl_easy_setopt(state->curl, CURLOPT_REDIR_PROTOCOLS, PROTOCOLS);
> +        if (curl_easy_setopt(state->curl, CURLOPT_PROTOCOLS, PROTOCOLS) ||
> +            curl_easy_setopt(state->curl, CURLOPT_REDIR_PROTOCOLS, PROTOCOLS)) {
> +            goto err;
> +        }
>  #endif
>  
>  #ifdef DEBUG_VERBOSE
> -        curl_easy_setopt(state->curl, CURLOPT_VERBOSE, 1);
> +        if (curl_easy_setopt(state->curl, CURLOPT_VERBOSE, 1)) {
> +            goto err;
> +        }
>  #endif
>      }
>  
>      state->s = s;
>  
>      return 0;
> +
> +err:
> +    curl_easy_cleanup(state->curl);
> +    state->curl = NULL;
> +    return -EIO;
>  }
>  
>  /* Called with s->mutex held.  */
> @@ -763,10 +785,11 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
>      }
>  
>      s->accept_range = false;
> -    curl_easy_setopt(state->curl, CURLOPT_NOBODY, 1);
> -    curl_easy_setopt(state->curl, CURLOPT_HEADERFUNCTION,
> -                     curl_header_cb);
> -    curl_easy_setopt(state->curl, CURLOPT_HEADERDATA, s);
> +    if (curl_easy_setopt(state->curl, CURLOPT_NOBODY, 1) ||
> +        curl_easy_setopt(state->curl, CURLOPT_HEADERFUNCTION, curl_header_cb) ||
> +        curl_easy_setopt(state->curl, CURLOPT_HEADERDATA, s)) {
> +        goto out;
> +    }
>      if (curl_easy_perform(state->curl))
>          goto out;
>      if (curl_easy_getinfo(state->curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &d)) {
> @@ -879,7 +902,10 @@ static void curl_setup_preadv(BlockDriverState *bs, CURLAIOCB *acb)
>  
>      snprintf(state->range, 127, "%" PRIu64 "-%" PRIu64, start, end);
>      trace_curl_setup_preadv(acb->bytes, start, state->range);
> -    curl_easy_setopt(state->curl, CURLOPT_RANGE, state->range);
> +    if (curl_easy_setopt(state->curl, CURLOPT_RANGE, state->range)) {
> +        curl_clean_state(state);
> +        goto out;
> +    }
>  
>      if (curl_multi_add_handle(s->multi, state->curl) != CURLM_OK) {
>          state->acb[0] = NULL;
> -- 
> 2.25.1
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|