On Thu, 5 Nov 2020 at 07:08, Chen Qun <kuhn.chen...@huawei.com> wrote:
>
> When 'j = icu->nr_sense – 1', the 'j < icu->nr_sense' condition is true,
> then 'j = icu->nr_sense', the'icu->init_sense[j]' has out-of-bounds access.
Yes, this is a bug...
> Maybe this could lead to some security problems.
...but it's not a security bug, because this device can't
be used with KVM, so it's not on the QEMU security boundary.
> hw/intc/rx_icu.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/intc/rx_icu.c b/hw/intc/rx_icu.c
> index 94e17a9dea..692a4c78e0 100644
> --- a/hw/intc/rx_icu.c
> +++ b/hw/intc/rx_icu.c
> @@ -308,11 +308,9 @@ static void rxicu_realize(DeviceState *dev, Error **errp)
> return;
> }
> for (i = j = 0; i < NR_IRQS; i++) {
> - if (icu->init_sense[j] == i) {
> + if (j < icu->nr_sense && icu->init_sense[j] == i) {
> icu->src[i].sense = TRG_LEVEL;
> - if (j < icu->nr_sense) {
> - j++;
> - }
> + j++;
> } else {
> icu->src[i].sense = TRG_PEDGE;
> }
This works, so:
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
but to be honest I think this would be more readable:
for (i = 0; i < NR_IRQS; i++) {
ice->src[i].sense = TRG_PEDGE;
}
for (i = 0; i < icu->nr_sense; i++) {
uint8_t irqno = icu->init_sense[i];
if (irqno < NR_IRQS) {
icu->src[irqno].sense = TRG_LEVEL;
}
}
so we first initialize everything to the default before
processing the init_sense array to identify which irqs
should be level-triggered. (It also means that the caller
doesn't have to ensure the input property is in sorted
order.)
thanks
-- PMM
