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

[Cornelia Huck]

On Tue, 15 May 2018 15:17:51 +0200
Halil Pasic <address@hidden> wrote:


> --------------------------------8<------------------------------------------------
> 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 | 10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 22df33b509..be433b0336 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) {
> @@ -1029,6 +1027,7 @@ static void virtio_ccw_notify(DeviceState *d, uint16_t 
> vector)
>                   css_adapter_interrupt(CSS_IO_ADAPTER_VIRTIO, 
> dev->thinint_isc);
>               }
>           } else {
> +            assert(vector < NR_CLASSIC_INDICATOR_BITS);
>               indicators = address_space_ldq(&address_space_memory,
>                                              dev->indicators->addr,
>                                              MEMTXATTRS_UNSPECIFIED,
> @@ -1042,12 +1041,11 @@ static void virtio_ccw_notify(DeviceState *d, 
> uint16_t vector)
>           if (!dev->indicators2) {
>               return;
>           }
> -        vector = 0;
>           indicators = address_space_ldq(&address_space_memory,
>                                          dev->indicators2->addr,
>                                          MEMTXATTRS_UNSPECIFIED,
>                                          NULL);
> -        indicators |= 1ULL << vector;
> +        indicators |= 1ULL;
>           address_space_stq(&address_space_memory, dev->indicators2->addr,
>                             indicators, MEMTXATTRS_UNSPECIFIED, NULL);
>           css_conditional_io_interrupt(sch);
> 

Looks sane.