| Message ID | 20230115155821.1534598-12-alexander.ivanov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | parallels: Refactor the code of images checks and fix a bug | expand |
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 > fprintf(stderr, "%s cluster %u is outside image\n", > fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i); > res->corruptions++;
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++; >
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) { fprintf(stderr, "%s cluster %u is outside image\n", fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i); res->corruptions++;