On 15 May 2018 at 13:00, Halil Pasic <pa...@linux.ibm.com> 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 <pa...@linux.ibm.com> > Date: Tue, 15 May 2018 13:57:44 +0200 > Subject: [PATCH] WIP: cleanup virtio notify > > Signed-off-by: Halil Pasic <pa...@linux.ibm.com> > --- > 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