Re: [PULL 02/17] block: Collapse padded I/O vecs exceeding IOV_MAX

[Peter Maydell]

On Mon, 5 Jun 2023 at 16:48, Hanna Czenczek <hreitz@redhat.com> wrote:
>
> When processing vectored guest requests that are not aligned to the
> storage request alignment, we pad them by adding head and/or tail
> buffers for a read-modify-write cycle.

Hi; Coverity complains (CID 1512819) that the assert added
in this commit is always true:

> @@ -1573,26 +1701,34 @@ static void bdrv_padding_destroy(BdrvRequestPadding 
> *pad)
>  static int bdrv_pad_request(BlockDriverState *bs,
>                              QEMUIOVector **qiov, size_t *qiov_offset,
>                              int64_t *offset, int64_t *bytes,
> +                            bool write,
>                              BdrvRequestPadding *pad, bool *padded,
>                              BdrvRequestFlags *flags)
>  {
>      int ret;
> +    struct iovec *sliced_iov;
> +    int sliced_niov;
> +    size_t sliced_head, sliced_tail;
>
>      bdrv_check_qiov_request(*offset, *bytes, *qiov, *qiov_offset, 
> &error_abort);
>
> -    if (!bdrv_init_padding(bs, *offset, *bytes, pad)) {
> +    if (!bdrv_init_padding(bs, *offset, *bytes, write, pad)) {
>          if (padded) {
>              *padded = false;
>          }
>          return 0;
>      }
>
> -    ret = qemu_iovec_init_extended(&pad->local_qiov, pad->buf, pad->head,
> -                                   *qiov, *qiov_offset, *bytes,
> -                                   pad->buf + pad->buf_len - pad->tail,
> -                                   pad->tail);
> +    sliced_iov = qemu_iovec_slice(*qiov, *qiov_offset, *bytes,
> +                                  &sliced_head, &sliced_tail,
> +                                  &sliced_niov);
> +
> +    /* Guaranteed by bdrv_check_qiov_request() */
> +    assert(*bytes <= SIZE_MAX);

This one. (The Coverity complaint is because SIZE_MAX for it is
UINT64_MAX and an int64_t can't possibly be bigger than that.)

Is this because the assert() is deliberately handling the case
of a 32-bit host where SIZE_MAX might not be UINT64_MAX ? Or was
the bound supposed to be SSIZE_MAX or INT64_MAX ?

Looking at bdrv_check_qiov_request(), it seems to check bytes
against BDRV_MAX_LENGTH, which is defined as something somewhere
near INT64_MAX. So on a 32-bit host I'm not sure that function
does guarantee that the bytes count is going to be less than
SIZE_MAX...

(CID 1512819)

thanks
-- PMM