diff mbox

[v3,05/10] qcow2: Fix refcount blocks beyond image end

Message ID 1408725104-17176-6-git-send-email-mreitz@redhat.com
State New
Headers show

Commit Message

Max Reitz Aug. 22, 2014, 4:31 p.m. UTC
If the qcow2 check function detects a refcount block located beyond the
image end, grow the image appropriately. This cannot break anything and
is the logical fix for such a case.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-refcount.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 46 insertions(+), 4 deletions(-)

Comments

Benoît Canet Aug. 22, 2014, 6:20 p.m. UTC | #1
On Fri, Aug 22, 2014 at 06:31:39PM +0200, Max Reitz wrote:
> If the qcow2 check function detects a refcount block located beyond the
> image end, grow the image appropriately. This cannot break anything and
> is the logical fix for such a case.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2-refcount.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 46 insertions(+), 4 deletions(-)
> 
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index babe6cb..1f0f44e 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -1505,7 +1505,8 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res,
>                             int64_t *nb_clusters)
>  {
>      BDRVQcowState *s = bs->opaque;
> -    int64_t i;
> +    int64_t i, size;
> +    int ret;
>  
>      for(i = 0; i < s->refcount_table_size; i++) {
>          uint64_t offset, cluster;
> @@ -1521,9 +1522,50 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res,
>          }
>  
>          if (cluster >= *nb_clusters) {
> -            fprintf(stderr, "ERROR refcount block %" PRId64
> -                    " is outside image\n", i);
> -            res->corruptions++;
> +            fprintf(stderr, "%s refcount block %" PRId64 " is outside image\n",
> +                    fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i);
> +
> +            if (fix & BDRV_FIX_ERRORS) {
> +                int64_t old_nb_clusters = *nb_clusters;
> +
> +                ret = bdrv_truncate(bs->file, offset + s->cluster_size);
> +                if (ret < 0) {
> +                    goto resize_fail;
> +                }
> +                size = bdrv_getlength(bs->file);
> +                if (size < 0) {
> +                    ret = size;
> +                    goto resize_fail;
> +                }
> +
> +                *nb_clusters = size_to_clusters(s, size);
> +                assert(*nb_clusters >= old_nb_clusters);
> +
> +                *refcount_table = g_try_realloc(*refcount_table,
> +                        *nb_clusters * sizeof(uint16_t));
> +                if (!*refcount_table) {
> +                    res->check_errors++;
> +                    return -ENOMEM;
> +                }
> +
> +                memset(*refcount_table + old_nb_clusters, 0,
> +                       (*nb_clusters - old_nb_clusters) * sizeof(uint16_t));
> +
> +                if (cluster >= *nb_clusters) {
> +                    ret = -EINVAL;
> +                    goto resize_fail;
> +                }
> +
> +                res->corruptions_fixed++;
> +                continue;
> +
> +resize_fail:
> +                res->corruptions++;
> +                fprintf(stderr, "ERROR could not resize image: %s\n",
> +                        strerror(-ret));
> +            } else {
> +                res->corruptions++;
> +            }
>              continue;
>          }
>  
> -- 
> 2.0.4
> 

Reviewed-by: Benoît Canet <benoit.canet@nodalink.com>
Eric Blake Aug. 26, 2014, 1:07 p.m. UTC | #2
On 08/22/2014 10:31 AM, Max Reitz wrote:
> If the qcow2 check function detects a refcount block located beyond the
> image end, grow the image appropriately. This cannot break anything and
> is the logical fix for such a case.

Does the testsuite cover this one? I didn't see it mentioned in patch 10/10.

> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2-refcount.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 46 insertions(+), 4 deletions(-)
>
Max Reitz Aug. 26, 2014, 6:06 p.m. UTC | #3
On 26.08.2014 15:07, Eric Blake wrote:
> On 08/22/2014 10:31 AM, Max Reitz wrote:
>> If the qcow2 check function detects a refcount block located beyond the
>> image end, grow the image appropriately. This cannot break anything and
>> is the logical fix for such a case.
> Does the testsuite cover this one? I didn't see it mentioned in patch 10/10.

No, it doesn't. I'll add it.

Max

>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   block/qcow2-refcount.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 46 insertions(+), 4 deletions(-)
>>
diff mbox

Patch

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index babe6cb..1f0f44e 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1505,7 +1505,8 @@  static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res,
                            int64_t *nb_clusters)
 {
     BDRVQcowState *s = bs->opaque;
-    int64_t i;
+    int64_t i, size;
+    int ret;
 
     for(i = 0; i < s->refcount_table_size; i++) {
         uint64_t offset, cluster;
@@ -1521,9 +1522,50 @@  static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res,
         }
 
         if (cluster >= *nb_clusters) {
-            fprintf(stderr, "ERROR refcount block %" PRId64
-                    " is outside image\n", i);
-            res->corruptions++;
+            fprintf(stderr, "%s refcount block %" PRId64 " is outside image\n",
+                    fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i);
+
+            if (fix & BDRV_FIX_ERRORS) {
+                int64_t old_nb_clusters = *nb_clusters;
+
+                ret = bdrv_truncate(bs->file, offset + s->cluster_size);
+                if (ret < 0) {
+                    goto resize_fail;
+                }
+                size = bdrv_getlength(bs->file);
+                if (size < 0) {
+                    ret = size;
+                    goto resize_fail;
+                }
+
+                *nb_clusters = size_to_clusters(s, size);
+                assert(*nb_clusters >= old_nb_clusters);
+
+                *refcount_table = g_try_realloc(*refcount_table,
+                        *nb_clusters * sizeof(uint16_t));
+                if (!*refcount_table) {
+                    res->check_errors++;
+                    return -ENOMEM;
+                }
+
+                memset(*refcount_table + old_nb_clusters, 0,
+                       (*nb_clusters - old_nb_clusters) * sizeof(uint16_t));
+
+                if (cluster >= *nb_clusters) {
+                    ret = -EINVAL;
+                    goto resize_fail;
+                }
+
+                res->corruptions_fixed++;
+                continue;
+
+resize_fail:
+                res->corruptions++;
+                fprintf(stderr, "ERROR could not resize image: %s\n",
+                        strerror(-ret));
+            } else {
+                res->corruptions++;
+            }
             continue;
         }