| Message ID | 20220712080133.4176971-3-yangxiaojuan@loongson.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix LoongArch coverity error and cpu name bug | expand |
On 7/12/22 13:31, Xiaojuan Yang wrote: > Fix coverity errors: > 1. In find_first_bit function, the 'size' argument need > 'unsigned long' type, so we add the suffix 'UL' on 'size' > argument when use the function. > 2. In expression 1ULL << irq, left shifting by more than > 63 bits has undefined behavior. And out-of-bounds access > error occured when 'irq' >= 64. So we add a condition to > avoid this. > > Fix coverity CID: 1489761 1489764 1489765 > > Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn> > --- > hw/intc/loongarch_pch_pic.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/hw/intc/loongarch_pch_pic.c b/hw/intc/loongarch_pch_pic.c > index 3c9814a3b4..779a087a03 100644 > --- a/hw/intc/loongarch_pch_pic.c > +++ b/hw/intc/loongarch_pch_pic.c > @@ -21,16 +21,20 @@ static void pch_pic_update_irq(LoongArchPCHPIC *s, uint64_t mask, int level) > if (level) { > val = mask & s->intirr & ~s->int_mask; > if (val) { > - irq = find_first_bit(&val, 64); > - s->intisr |= 0x1ULL << irq; > - qemu_set_irq(s->parent_irq[s->htmsi_vector[irq]], 1); > + irq = find_first_bit(&val, 64UL); > + if (irq < 64) { > + s->intisr |= 0x1ULL << irq; > + qemu_set_irq(s->parent_irq[s->htmsi_vector[irq]], 1); > + } > } > } else { > val = mask & s->intisr; > if (val) { > - irq = find_first_bit(&val, 64); > - s->intisr &= ~(0x1ULL << irq); > - qemu_set_irq(s->parent_irq[s->htmsi_vector[irq]], 0); > + irq = find_first_bit(&val, 64UL); > + if (irq < 64) { > + s->intisr &= ~(0x1ULL << irq); > + qemu_set_irq(s->parent_irq[s->htmsi_vector[irq]], 0); > + } This needs to be rewritten. I mentioned this multiple times during review, but this instance seems to have slipped through anyway. You absolutely cannot use find_first_bit etc on raw 'unsigned long' -- bitmap.h functions may only be used on DECLARE_BITMAP objects. That said, there is no point in using any of the bitmap.h functions here. You should simply use uint64_t val, and ctz64() instead of find_first_bit(). r~
diff --git a/hw/intc/loongarch_pch_pic.c b/hw/intc/loongarch_pch_pic.c index 3c9814a3b4..779a087a03 100644 --- a/hw/intc/loongarch_pch_pic.c +++ b/hw/intc/loongarch_pch_pic.c @@ -21,16 +21,20 @@ static void pch_pic_update_irq(LoongArchPCHPIC *s, uint64_t mask, int level) if (level) { val = mask & s->intirr & ~s->int_mask; if (val) { - irq = find_first_bit(&val, 64); - s->intisr |= 0x1ULL << irq; - qemu_set_irq(s->parent_irq[s->htmsi_vector[irq]], 1); + irq = find_first_bit(&val, 64UL); + if (irq < 64) { + s->intisr |= 0x1ULL << irq; + qemu_set_irq(s->parent_irq[s->htmsi_vector[irq]], 1); + } } } else { val = mask & s->intisr; if (val) { - irq = find_first_bit(&val, 64); - s->intisr &= ~(0x1ULL << irq); - qemu_set_irq(s->parent_irq[s->htmsi_vector[irq]], 0); + irq = find_first_bit(&val, 64UL); + if (irq < 64) { + s->intisr &= ~(0x1ULL << irq); + qemu_set_irq(s->parent_irq[s->htmsi_vector[irq]], 0); + } } } }
Fix coverity errors: 1. In find_first_bit function, the 'size' argument need 'unsigned long' type, so we add the suffix 'UL' on 'size' argument when use the function. 2. In expression 1ULL << irq, left shifting by more than 63 bits has undefined behavior. And out-of-bounds access error occured when 'irq' >= 64. So we add a condition to avoid this. Fix coverity CID: 1489761 1489764 1489765 Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn> --- hw/intc/loongarch_pch_pic.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)