Re: [PATCH v2] irqchip/riscv-imsic: Reject overlapping group index fields

[Anup Patel]

On Sun, Sep 20, 2026 at 6:38 PM Pengpeng Hou <hppiscas@xxxxxxx> wrote:
>
> The IMSIC address layout places the group index above the page offset,
> guest index and HART index. imsic_parse_fwnode() checks the individual
> field widths and the upper end of the group field, but does not check
> that its lower end is above the other fields.
>
> Reject a nonzero group field whose shift overlaps the lower fields.
> Otherwise a firmware-supplied layout can assign the same address bit to
> different indices when MSI target addresses are constructed.
>
> Keep the existing upper-bound check unchanged. The new lower-bound
> check applies only when group index bits are present.
>
> The issue was found by our static-analysis tool.
>
> Fixes: 21a8f8a0eb35 ("irqchip: Add RISC-V incoming MSI controller early driver")
> Assisted-by: gpt 5
> Signed-off-by: Pengpeng Hou <hppiscas@xxxxxxx>

LGTM.

Reviewed-by: Anup Patel <anup@xxxxxxxxxxxxxx>

Thanks,
Anup


> ---
> Changes since v1: https://lore.kernel.org/r/caplitmus-rv.bc36e3a0ebcb4a01838171f27625004b.imsic-overlap@xxxxxxx/
> - Limit the change to the lower-bound check discussed with Anup.
> - Keep the existing upper-bound check unchanged.
> - Send this as a standalone patch, without the zero-width-group change.
> - Do not change APLIC configuration or require the earlier series.
>
>  drivers/irqchip/irq-riscv-imsic-state.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/irqchip/irq-riscv-imsic-state.c b/drivers/irqchip/irq-riscv-imsic-state.c
> index b8d1bbb..a7b241f 100644
> --- a/drivers/irqchip/irq-riscv-imsic-state.c
> +++ b/drivers/irqchip/irq-riscv-imsic-state.c
> @@ -739,6 +739,14 @@ static int __init imsic_parse_fwnode(struct fwnode_handle *fwnode,
>                 return -EINVAL;
>         }
>
> +       /* Group index bits must not overlap guest and HART index bits. */
> +       i = IMSIC_MMIO_PAGE_SHIFT + global->guest_index_bits +
> +           global->hart_index_bits;
> +       if (global->group_index_bits && global->group_index_shift < i) {
> +               pr_err("%pfwP: group index shift too small\n", fwnode);
> +               return -EINVAL;
> +       }
> +
>         /* Sanity check group index shift */
>         i = global->group_index_bits + global->group_index_shift - 1;
>         if (i >= BITS_PER_LONG) {
>
> base-commit: 518e5b794c06c0f0eb40df3e202274a66202c137
>