Re: [PATCH 03/14] qemu-io: Allow larger write zeroes under no fallback

[Vladimir Sementsov-Ogievskiy]

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