[PATCH v4] block: Fix integer promotion error in bdrv_getlength()

[PATCH v4] block: Fix integer promotion error in bdrv_getlength()

[Eric Blake]

Back in 2015, we attempted to fix error reporting for images that
claimed to have more than INT64_MAX/512 sectors, but due to the type
promotions caused by BDRV_SECTOR_SIZE being unsigned, this
inadvertently forces all negative ret values to be slammed into -EFBIG
rather than the original error.  While we're at it, we can avoid the
confusing ?: by spelling the logic more directly.

Fixes: 4a9c9ea0d3
Reported-by: Guoyi Tu <tu.guoyi@h3c.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 56bacc9e9f13..2fd932154e12 100644
--- a/block.c
+++ b/block.c
@@ -5091,8 +5091,13 @@ int64_t bdrv_getlength(BlockDriverState *bs)
 {
     int64_t ret = bdrv_nb_sectors(bs);

-    ret = ret > INT64_MAX / BDRV_SECTOR_SIZE ? -EFBIG : ret;
-    return ret < 0 ? ret : ret * BDRV_SECTOR_SIZE;
+    if (ret < 0) {
+        return ret;
+    }
+    if (ret > INT64_MAX / BDRV_SECTOR_SIZE) {
+        return -EFBIG;
+    }
+    return ret * BDRV_SECTOR_SIZE;
 }

 /* return 0 as number of sectors if no device present or error */
-- 
2.28.0

Re: [PATCH v4] block: Fix integer promotion error in bdrv_getlength()

[Alberto Garcia]

On Thu 05 Nov 2020 04:51:22 PM CET, Eric Blake wrote:
> Back in 2015, we attempted to fix error reporting for images that
> claimed to have more than INT64_MAX/512 sectors, but due to the type
> promotions caused by BDRV_SECTOR_SIZE being unsigned, this
> inadvertently forces all negative ret values to be slammed into -EFBIG
> rather than the original error.  While we're at it, we can avoid the
> confusing ?: by spelling the logic more directly.
>
> Fixes: 4a9c9ea0d3
> Reported-by: Guoyi Tu <tu.guoyi@h3c.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

Re: [PATCH v4] block: Fix integer promotion error in bdrv_getlength()

[Max Reitz]

On 05.11.20 16:51, Eric Blake wrote:
> Back in 2015, we attempted to fix error reporting for images that
> claimed to have more than INT64_MAX/512 sectors, but due to the type
> promotions caused by BDRV_SECTOR_SIZE being unsigned, this
> inadvertently forces all negative ret values to be slammed into -EFBIG
> rather than the original error.  While we're at it, we can avoid the
> confusing ?: by spelling the logic more directly.
> 
> Fixes: 4a9c9ea0d3
> Reported-by: Guoyi Tu <tu.guoyi@h3c.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>   block.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)

Thanks, applied to my block branch (replacing the original patch):

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max