On 23.08.2022 09:23, Alexander Ivanov wrote:
On 23.08.2022 08:58, Vladimir Sementsov-Ogievskiy wrote:
On 8/22/22 12:05, 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).
Signed-off-by: Alexander Ivanov <alexander.iva...@virtuozzo.com>
---
block/parallels.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/block/parallels.c b/block/parallels.c
index a229c06f25..c245ca35cd 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;
@@ -811,6 +812,19 @@ static int parallels_open(BlockDriverState *bs,
QDict *options, int flags,
}
}
+ file_size = bdrv_getlength(bs->file->bs);
+ if (file_size < 0) {
+ ret = file_size;
+ goto fail;
+ }
+
+ file_size >>= BDRV_SECTOR_BITS;
+ if (s->data_end > file_size && !(flags & BDRV_O_CHECK)) {
+ error_setg(errp, "parallels: Offset in BAT is out of image");
+ ret = -EINVAL;
+ goto fail;
+ }
If image is unaligned to sector size, and image size is less than
s->data_end, but the difference itself is less than sector, the error
message would be misleading.
Should we consider "file_size = DIV_ROUND_UP(file_size,
BDRV_SECTOR_SIZE)" instead of "file_size >>= BDRV_SECTOR_BITS"?
It's hardly possible to get such image on valid scenarios with Qemu
(keeping in mind bdrv_truncate() call in parallels_close()). But it
still may be possible to have such images produced by another
software or by some failure path.
I think you are right, it would be better to align image size up to
sector size.
I would say that we need to align not on sector size but on cluster size.
That would worth additional check.
