From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55567) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XiJeT-0003sf-Bw for qemu-devel@nongnu.org; Sun, 26 Oct 2014 04:58:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XiJeN-0001aO-7E for qemu-devel@nongnu.org; Sun, 26 Oct 2014 04:58:17 -0400 Received: from szxga02-in.huawei.com ([119.145.14.65]:9546) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XiJeM-0001a6-I8 for qemu-devel@nongnu.org; Sun, 26 Oct 2014 04:58:11 -0400 Message-ID: <544CB78A.3090403@huawei.com> Date: Sun, 26 Oct 2014 16:57:46 +0800 From: Gonglei MIME-Version: 1.0 References: <1414312958-21967-1-git-send-email-rjones@redhat.com> <1414312958-21967-2-git-send-email-rjones@redhat.com> In-Reply-To: <1414312958-21967-2-git-send-email-rjones@redhat.com> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v1 repost 2] block/curl: Improve type safety of s->timeout. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Richard W.M. Jones" Cc: kwolf@redhat.com, lersek@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com On 2014/10/26 16:42, Richard W.M. Jones wrote: > qemu_opt_get_number returns a uint64_t, and curl_easy_setopt expects a > long (not an int). > > Store the timeout (which is a positive number of seconds) as a > uint64_t. Check that the number given by the user is reasonable. > Cast it to long before calling curl_easy_setopt. > > Example error message after this change has been applied: > > $ ./qemu-img create -f qcow2 /tmp/test.qcow2 \ > -b 'json: { "file.driver":"https", > "file.url":"https://foo/bar", > "file.timeout":-1 }' > qemu-img: /tmp/test.qcow2: Could not open 'json: { "file.driver":"https", "file.url":"https://foo/bar", "file.timeout":-1 }': timeout parameter is too large or negative: Invalid argument > > Signed-off-by: Richard W.M. Jones > Reviewed-by: Laszlo Ersek > --- > block/curl.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/block/curl.c b/block/curl.c > index b4157cc..2b40802 100644 > --- a/block/curl.c > +++ b/block/curl.c > @@ -112,7 +112,7 @@ typedef struct BDRVCURLState { > char *url; > size_t readahead_size; > bool sslverify; > - int timeout; > + uint64_t timeout; > char *cookie; > bool accept_range; > AioContext *aio_context; > @@ -390,7 +390,7 @@ static CURLState *curl_init_state(BlockDriverState *bs, BDRVCURLState *s) > if (s->cookie) { > curl_easy_setopt(state->curl, CURLOPT_COOKIE, s->cookie); > } > - curl_easy_setopt(state->curl, CURLOPT_TIMEOUT, s->timeout); > + 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); > @@ -546,6 +546,10 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags, > > s->timeout = qemu_opt_get_number(opts, CURL_BLOCK_OPT_TIMEOUT, > CURL_TIMEOUT_DEFAULT); > + if (s->timeout > 100000) { why 100000? And it's a magic number. Best regards, -Gonglei > + error_setg(errp, "timeout parameter is too large or negative"); > + goto out_noclean; > + } > > s->sslverify = qemu_opt_get_bool(opts, CURL_BLOCK_OPT_SSLVERIFY, true); >