When writing zeroes can fall back to a slow write, permitting an
overly large request can become an amplification denial of service
attack in triggering a large amount of work from a small request.  But
the whole point of the no fallback flag is to quickly determine if
writing an entire device to zero can be done quickly (such as when it
is already known that the device started with zero contents); in those
cases, artificially capping things at 2G in qemu-io itself doesn't
help us.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 qemu-io-cmds.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 954955c12fb9..45a957093369 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -603,10 +603,6 @@ static int do_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
         .done   = false,
     };

-    if (bytes > INT_MAX) {
-        return -ERANGE;
-    }
-
     co = qemu_coroutine_create(co_pwrite_zeroes_entry, &data);
     bdrv_coroutine_enter(blk_bs(blk), co);
     while (!data.done) {
@@ -1160,8 +1156,9 @@ static int write_f(BlockBackend *blk, int argc, char **argv)
     if (count < 0) {
         print_cvtnum_err(count, argv[optind]);
         return count;
-    } else if (count > BDRV_REQUEST_MAX_BYTES) {
-        printf("length cannot exceed %" PRIu64 ", given %s\n",
+    } else if (count > BDRV_REQUEST_MAX_BYTES &&
+               !(flags & BDRV_REQ_NO_FALLBACK)) {
+        printf("length cannot exceed %" PRIu64 " without -n, given %s\n",
                (uint64_t)BDRV_REQUEST_MAX_BYTES, argv[optind]);
         return -EINVAL;
     }
-- 
2.33.1

04.12.2021 02:15, Eric Blake wrote:
> When writing zeroes can fall back to a slow write, permitting an
> overly large request can become an amplification denial of service
> attack in triggering a large amount of work from a small request.  But
> the whole point of the no fallback flag is to quickly determine if
> writing an entire device to zero can be done quickly (such as when it
> is already known that the device started with zero contents); in those
> cases, artificially capping things at 2G in qemu-io itself doesn't
> help us.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>   qemu-io-cmds.c | 9 +++------
>   1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index 954955c12fb9..45a957093369 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -603,10 +603,6 @@ static int do_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
>           .done   = false,
>       };
> 
> -    if (bytes > INT_MAX) {
> -        return -ERANGE;
> -    }
> -
>       co = qemu_coroutine_create(co_pwrite_zeroes_entry, &data);
>       bdrv_coroutine_enter(blk_bs(blk), co);
>       while (!data.done) {
> @@ -1160,8 +1156,9 @@ static int write_f(BlockBackend *blk, int argc, char **argv)
>       if (count < 0) {
>           print_cvtnum_err(count, argv[optind]);
>           return count;
> -    } else if (count > BDRV_REQUEST_MAX_BYTES) {
> -        printf("length cannot exceed %" PRIu64 ", given %s\n",
> +    } else if (count > BDRV_REQUEST_MAX_BYTES &&
> +               !(flags & BDRV_REQ_NO_FALLBACK)) {
> +        printf("length cannot exceed %" PRIu64 " without -n, given %s\n",

Actually, I don't see the reason to restrict qemu-io which is mostly a testing tool in this way. What if I want to test data reqeust > 2G, why not?

So, we restring user in testing, but don't avoid any kind of DOS: bad gues can always modify the code and rebuild qemu-io to overcome the restriction.

But I don't really care of it, patch is not wrong:

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>


-- 
Best regards,
Vladimir