| Message ID | 20220808120734.1168314-8-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 08.08.2022 14:07, Alexander Ivanov wrote: > When an image is opened, data_end field in BDRVParallelsState > is setted as the biggest offset in the BAT plus cluster size. > If there is a corrupted offset pointing outside the image, > the image size increase accordingly. It potentially leads > to attempts to create a file size of petabytes. > > Set the data_end field with the original file size if the image > was opened for checking and repairing purposes or raise an error. > > Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> > --- > block/parallels.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/block/parallels.c b/block/parallels.c > index 3cb5452613..72cf7499c1 100644 > --- a/block/parallels.c > +++ b/block/parallels.c > @@ -811,6 +811,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, > BDRVParallelsState *s = bs->opaque; > ParallelsHeader ph; > int ret, size, i; > + int64_t file_size; > QemuOpts *opts = NULL; > Error *local_err = NULL; > char *buf; > @@ -890,6 +891,22 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, > } > } > > + file_size = bdrv_getlength(bs->file->bs); > + if (file_size < 0) { > + goto fail; > + } > + > + file_size >>= BDRV_SECTOR_BITS; > + if (s->data_end > file_size) { > + if (flags & BDRV_O_CHECK) { > + s->data_end = file_size; > + } else { > + error_setg(errp, "parallels: Offset in BAT is out of image"); > + ret = -EINVAL; > + goto fail; > + } > + } > + > if (le32_to_cpu(ph.inuse) == HEADER_INUSE_MAGIC) { > /* Image was not closed correctly. The check is mandatory */ > s->header_unclean = true; for me it would be more logical to have this check inside the loop, calculating data_offset. Though this is personal.
diff --git a/block/parallels.c b/block/parallels.c index 3cb5452613..72cf7499c1 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -811,6 +811,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, BDRVParallelsState *s = bs->opaque; ParallelsHeader ph; int ret, size, i; + int64_t file_size; QemuOpts *opts = NULL; Error *local_err = NULL; char *buf; @@ -890,6 +891,22 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, } } + file_size = bdrv_getlength(bs->file->bs); + if (file_size < 0) { + goto fail; + } + + file_size >>= BDRV_SECTOR_BITS; + if (s->data_end > file_size) { + if (flags & BDRV_O_CHECK) { + s->data_end = file_size; + } else { + error_setg(errp, "parallels: Offset in BAT is out of image"); + ret = -EINVAL; + goto fail; + } + } + if (le32_to_cpu(ph.inuse) == HEADER_INUSE_MAGIC) { /* Image was not closed correctly. The check is mandatory */ s->header_unclean = true;
When an image is opened, data_end field in BDRVParallelsState is setted as the biggest offset in the BAT plus cluster size. If there is a corrupted offset pointing outside the image, the image size increase accordingly. It potentially leads to attempts to create a file size of petabytes. Set the data_end field with the original file size if the image was opened for checking and repairing purposes or raise an error. Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> --- block/parallels.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)