From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40096) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XNSHh-0005Vg-MD for qemu-devel@nongnu.org; Fri, 29 Aug 2014 15:56:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XNSHZ-0008E4-8A for qemu-devel@nongnu.org; Fri, 29 Aug 2014 15:56:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:19183) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XNSHZ-0008D7-08 for qemu-devel@nongnu.org; Fri, 29 Aug 2014 15:56:25 -0400 Message-ID: <5400DAE0.7090602@redhat.com> Date: Fri, 29 Aug 2014 21:56:16 +0200 From: Max Reitz MIME-Version: 1.0 References: <1409170706-24465-1-git-send-email-mreitz@redhat.com> <1409170706-24465-6-git-send-email-mreitz@redhat.com> <5400D989.9070801@redhat.com> In-Reply-To: <5400D989.9070801@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 05/10] qcow2: Fix refcount blocks beyond image end List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: Kevin Wolf , Stefan Hajnoczi , =?UTF-8?B?QmVub8OudCBDYW5ldA==?= On 29.08.2014 21:50, Eric Blake wrote: > On 08/27/2014 02:18 PM, Max Reitz wrote: >> If the qcow2 check function detects a refcount block located beyond th= e >> image end, grow the image appropriately. This cannot break anything an= d >> is the logical fix for such a case. >> >> Signed-off-by: Max Reitz >> --- >> block/qcow2-refcount.c | 62 ++++++++++++++++++++++++++++++++++++++++= ++++++---- >> 1 file changed, 58 insertions(+), 4 deletions(-) >> >> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c >> index babe6cb..394a402 100644 >> --- a/block/qcow2-refcount.c >> +++ b/block/qcow2-refcount.c >> @@ -1505,7 +1505,8 @@ static int check_refblocks(BlockDriverState *bs,= BdrvCheckResult *res, >> int64_t *nb_clusters) >> { >> BDRVQcowState *s =3D bs->opaque; >> - int64_t i; >> + int64_t i, size; >> + int ret; >> =20 >> for(i =3D 0; i < s->refcount_table_size; i++) { > Is it worth fixing up the whitespace on this 'for' at any point in the > series? In v1 of this series I fixed several preexisting coding style issues.=20 However, when Beno=C3=AEt requested smaller diffs (and keep the coding st= yle=20 fixes outside of the code moving patches), I decided to throw all of=20 those out. I would have to write an explicit coding style fix patch, but=20 it turned out there are a lot of style issues in qcow2-refcount.c. Max >> + if (fix & BDRV_FIX_ERRORS) { >> + int64_t old_nb_clusters =3D *nb_clusters; >> + >> + if (offset + s->cluster_size < offset || >> + offset + s->cluster_size > INT64_MAX) > [1] > >> + >> + *refcount_table =3D g_try_realloc(*refcount_table, >> + *nb_clusters * sizeof(uint16_t)); > I was about to complain that this multiply could overflow if > *nb_clusters is more than 2**62 bits, until I double checked that due t= o > the limit checking at [1], we know *nb_clusters is narrower. > > Reviewed-by: Eric Blake >