| Message ID | 20220825143109.176582-2-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 25.08.2022 16:31, Alexander Ivanov wrote: > data_end field in BDRVParallelsState is set to the biggest offset present > in BAT. If this offset is outside of the image, any further write will create > the cluster at this offset and/or the image will be truncated to this > offset on close. This is definitely not correct. > Raise an error in parallels_open() if data_end points outside the image and > it is not a check (let the check to repaire the image). > Set data_end to the end of the cluster with the last correct offset. > > 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 a229c06f25..93bc2750ef 100644 > --- a/block/parallels.c > +++ b/block/parallels.c > @@ -732,6 +732,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; > @@ -742,6 +743,12 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, > return -EINVAL; > } > > + file_size = bdrv_getlength(bs->file->bs); > + if (file_size < 0) { > + return -EINVAL; > + } > + file_size >>= BDRV_SECTOR_BITS; > + > ret = bdrv_pread(bs->file, 0, sizeof(ph), &ph, 0); > if (ret < 0) { > goto fail; > @@ -806,6 +813,16 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, > > for (i = 0; i < s->bat_size; i++) { > int64_t off = bat2sect(s, i); > + if (off >= file_size) { > + if (flags & BDRV_O_CHECK) { > + continue; > + } > + error_setg(errp, "parallels: Offset %" PRIi64 " in BAT[%d] entry " > + "is larger than file size (%" PRIi64 ")", > + off, i, file_size); > + ret = -EINVAL; > + goto fail; > + } > if (off >= s->data_end) { > s->data_end = off + s->tracks; > } with string length fixes in the commit message (more that 74 chars) Reviewed-by: Denis V. Lunev <den@openvz.org>
diff --git a/block/parallels.c b/block/parallels.c index a229c06f25..93bc2750ef 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -732,6 +732,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; @@ -742,6 +743,12 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, return -EINVAL; } + file_size = bdrv_getlength(bs->file->bs); + if (file_size < 0) { + return -EINVAL; + } + file_size >>= BDRV_SECTOR_BITS; + ret = bdrv_pread(bs->file, 0, sizeof(ph), &ph, 0); if (ret < 0) { goto fail; @@ -806,6 +813,16 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, for (i = 0; i < s->bat_size; i++) { int64_t off = bat2sect(s, i); + if (off >= file_size) { + if (flags & BDRV_O_CHECK) { + continue; + } + error_setg(errp, "parallels: Offset %" PRIi64 " in BAT[%d] entry " + "is larger than file size (%" PRIi64 ")", + off, i, file_size); + ret = -EINVAL; + goto fail; + } if (off >= s->data_end) { s->data_end = off + s->tracks; }
data_end field in BDRVParallelsState is set to the biggest offset present in BAT. If this offset is outside of the image, any further write will create the cluster at this offset and/or the image will be truncated to this offset on close. This is definitely not correct. Raise an error in parallels_open() if data_end points outside the image and it is not a check (let the check to repaire the image). Set data_end to the end of the cluster with the last correct offset. Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> --- block/parallels.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)