[Top][All Lists]

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

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

From: Denis V. Lunev
Subject: Re: [PATCH v3 1/8] parallels: Out of image offset in BAT leads to image inflation
Date: Wed, 17 Aug 2022 21:27:16 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0

On 17.08.2022 21:13, Vladimir Sementsov-Ogievskiy wrote:
On 8/15/22 12:02, 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 and should be fixed.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
v2: No change.
v3: Fix commit message.

  block/parallels.c | 17 +++++++++++++++++
  1 file changed, 17 insertions(+)

diff --git a/block/parallels.c b/block/parallels.c
index a229c06f25..a76cf9d993 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,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;

Hm. but with this, any further allocation may lead to twice-allocted clusters, as you just modify s->data_end, but havn't yet fixed the BAT entry.. It seems unsafe. Or what I miss?

if O_CHECK is specified, we are going to execute parallels_co_check which
will correctly handle this. In the other case (normal open) we will
face the error, which is pretty much correct under this logic.

+        } 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;

reply via email to

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