Re: [qemu-s390x] virtio-ccw.c vs larger VIRTIO_QUEUE_MAX (coverity warning CID 1390619)

[Peter Maydell]

On 15 May 2018 at 13:00, Halil Pasic <address@hidden> wrote:
> To sum it up, my take on the whole is the diff below. I can convert
> it to a proper patch if we agree that's the way to go.
>
> From: Halil Pasic <address@hidden>
> Date: Tue, 15 May 2018 13:57:44 +0200
> Subject: [PATCH] WIP: cleanup virtio notify
>
> Signed-off-by: Halil Pasic <address@hidden>
> ---
>  hw/s390x/virtio-ccw.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index e51fbefd23..22078605d1 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -1003,10 +1003,8 @@ static void virtio_ccw_notify(DeviceState *d,
> uint16_t vector)
>      SubchDev *sch = ccw_dev->sch;
>      uint64_t indicators;
>
> -    /* queue indicators + secondary indicators */
> -    if (vector >= VIRTIO_QUEUE_MAX + 64) {
> -        return;
> -    }
> +    /* vector == VIRTIO_QUEUE_MAX means configuration change */
> +    assert(vector <= VIRTIO_QUEUE_MAX);
>
>      if (vector < VIRTIO_QUEUE_MAX) {
>          if (!dev->indicators) {

This will not be sufficient to satisfy Coverity, because
VIRTIO_QUEUE_MAX is 1024, and that is way too big to use as
a shift count, and that's the only check that Coverity can see
on the code path leading into the "1ULL << vector" code.

If it's not possible to get here with a vector that's 64 or more,
you need to
    assert(vector < NR_CLASSIC_INDICATOR_BITS);
somewhere.

thanks
-- PMM