[Qemu-devel] [PATCH for-2.10] vmdk: Fix error handling/reporting of vmdk_check

[Qemu-devel] [PATCH for-2.10] vmdk: Fix error handling/reporting of vmdk_check

[Fam Zheng]

Errors from the callees must be captured and propagated to our caller,
ensure this for both find_extent() and bdrv_getlength().

Reported-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/vmdk.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 0fc97391a6..c665bcc977 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2236,6 +2236,7 @@ static int vmdk_check(BlockDriverState *bs, BdrvCheckResult *result,
             fprintf(stderr,
                     "ERROR: could not find extent for sector %" PRId64 "\n",
                     sector_num);
+            ret = -EINVAL;
             break;
         }
         ret = get_cluster_offset(bs, extent, NULL,
@@ -2247,19 +2248,28 @@ static int vmdk_check(BlockDriverState *bs, BdrvCheckResult *result,
                     PRId64 "\n", sector_num);
             break;
         }
-        if (ret == VMDK_OK &&
-            cluster_offset >= bdrv_getlength(extent->file->bs))
-        {
-            fprintf(stderr,
-                    "ERROR: cluster offset for sector %"
-                    PRId64 " points after EOF\n", sector_num);
-            break;
+        if (ret == VMDK_OK) {
+            int64_t extent_len = bdrv_getlength(extent->file->bs);
+            if (extent_len < 0) {
+                fprintf(stderr,
+                        "ERROR: could not get extent file length for sector %"
+                        PRId64 "\n", sector_num);
+                ret = extent_len;
+                break;
+            }
+            if (cluster_offset >= extent_len) {
+                fprintf(stderr,
+                        "ERROR: cluster offset for sector %"
+                        PRId64 " points after EOF\n", sector_num);
+                ret = -EINVAL;
+                break;
+            }
         }
         sector_num += extent->cluster_sectors;
     }
 
     result->corruptions++;
-    return 0;
+    return ret;
 }
 
 static ImageInfoSpecific *vmdk_get_specific_info(BlockDriverState *bs)
-- 
2.13.3

Re: [Qemu-devel] [PATCH for-2.10] vmdk: Fix error handling/reporting of vmdk_check

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1021 bytes --]

On 08/04/2017 09:09 AM, Fam Zheng wrote:
> Errors from the callees must be captured and propagated to our caller,
> ensure this for both find_extent() and bdrv_getlength().
> 
> Reported-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block/vmdk.c | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 

> +        if (ret == VMDK_OK) {
> +            int64_t extent_len = bdrv_getlength(extent->file->bs);
> +            if (extent_len < 0) {
> +                fprintf(stderr,
> +                        "ERROR: could not get extent file length for sector %"
> +                        PRId64 "\n", sector_num);
> +                ret = extent_len;

Pre-existing - our use of fprintf() is not ideal.  But this patch
doesn't make it worse.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH for-2.10] vmdk: Fix error handling/reporting of vmdk_check

[Kevin Wolf]

Am 04.08.2017 um 16:09 hat Fam Zheng geschrieben:
> Errors from the callees must be captured and propagated to our caller,
> ensure this for both find_extent() and bdrv_getlength().
> 
> Reported-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Fam Zheng <famz@redhat.com>

Thanks, applied to the block branch.

Kevin