Re: [PATCH v8 11/11] parallels: Incorrect condition in out-of-image check

[Alexander Ivanov]

On 18.01.2023 15:46, Hanna Czenczek wrote:
> On 15.01.23 16:58, Alexander Ivanov wrote:
> > All the offsets in the BAT must be lower than the file size.
> > Fix the check condition for correct check.
> >
> > Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> > Reviewed-by: Denis V. Lunev <den@openvz.org>
> > ---
> >   block/parallels.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/block/parallels.c b/block/parallels.c
> > index 621dbf623a..eda3fb558d 100644
> > --- a/block/parallels.c
> > +++ b/block/parallels.c
> > @@ -455,7 +455,7 @@ static int 
> > parallels_check_outside_image(BlockDriverState *bs,
> >       high_off = 0;
> >       for (i = 0; i < s->bat_size; i++) {
> >           off = bat2sect(s, i) << BDRV_SECTOR_BITS;
> > -        if (off > size) {
> > +        if (off >= size) {
>
> Should this not be the even stricter `off + s->cluster_size > size` 
> instead, or is it possible to have partial clusters at the image end?
>
> Hanna
'off' is aligned* on the cluster size, so these conditions are 
equivalent, but I agree, your condition is more idiomatic.

* It works for the new image format. In the old one there could be 
entries in the BAT, pointing to unaligned clusters. There will be a 
separate check for unaligned clusters.
> >               fprintf(stderr, "%s cluster %u is outside image\n",
> >                       fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i);
> >               res->corruptions++;