qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v5 1/9] parallels: Out of image offset in BAT leads to image


From: Denis V. Lunev
Subject: Re: [PATCH v5 1/9] parallels: Out of image offset in BAT leads to image inflation
Date: Tue, 23 Aug 2022 11:20:59 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0

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.ivanov@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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]