[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] block/curl.c: Check error return from curl_easy_setopt()
|
From: |
Daniel P . Berrangé |
|
Subject: |
Re: [PATCH] block/curl.c: Check error return from curl_easy_setopt() |
|
Date: |
Fri, 28 Jan 2022 17:50:18 +0000 |
|
User-agent: |
Mutt/2.1.5 (2021-12-30) |
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 :|