Re: [Qemu-devel] [PATCH v4 05/10] qcow2: Fix refcount blocks beyond image end

[Max Reitz]

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 the
> > image end, grow the image appropriately. This cannot break anything and
> > is the logical fix for such a case.
> >
> > Signed-off-by: Max Reitz <address@hidden>
> > ---
> >   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 = bs->opaque;
> > -    int64_t i;
> > +    int64_t i, size;
> > +    int ret;
> >   
> >       for(i = 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. 
However, when Benoît requested smaller diffs (and keep the coding style 
fixes outside of the code moving patches), I decided to throw all of 
those out. I would have to write an explicit coding style fix patch, but 
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 = *nb_clusters;
> > +
> > +                if (offset + s->cluster_size < offset ||
> > +                    offset + s->cluster_size > INT64_MAX)
> [1]
>
> > +
> > +                *refcount_table = 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 to
> the limit checking at [1], we know *nb_clusters is narrower.
>
> Reviewed-by: Eric Blake <address@hidden>