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

[Anup Patel]

On Sun, Sep 6, 2026 at 1:39 PM Pengpeng Hou <hppiscas@xxxxxxx> wrote:
>
> The IMSIC parser validates index widths but does not check that a
> nonzero group field starts above the page, guest and hart fields.
> A malformed layout can therefore assign one address bit to two fields.
>
> Require the group shift to be at least the end of the lower fields.
> Check the upper bound using subtraction after validating the shift, so
> the bound check cannot wrap when processing a firmware-supplied value.
>
> The issue was found by our static-analysis tool and manually reviewed.
>
> Fixes: 21a8f8a0eb35 ("irqchip: Add RISC-V incoming MSI controller early driver")
> Assisted-by: GPT-5
> Signed-off-by: Pengpeng Hou <hppiscas@xxxxxxx>
> ---
>
> diff --git a/drivers/irqchip/irq-riscv-imsic-state.c b/drivers/irqchip/irq-riscv-imsic-state.c
> index 44b9c34c..c6eb16ae 100644
> --- a/drivers/irqchip/irq-riscv-imsic-state.c
> +++ b/drivers/irqchip/irq-riscv-imsic-state.c
> @@ -742,8 +742,16 @@ static int __init imsic_parse_fwnode(struct fwnode_handle *fwnode,
>
>         /* Sanity check group index shift */
>         if (global->group_index_bits) {
> -               i = global->group_index_bits + global->group_index_shift - 1;
> -               if (i >= BITS_PER_LONG) {
> +               i = IMSIC_MMIO_PAGE_SHIFT + global->guest_index_bits +
> +                   global->hart_index_bits;
> +               if (global->group_index_shift < i) {
> +                       pr_err("%pfwP: group index shift too small\n", fwnode);
> +                       return -EINVAL;
> +               }
> +
> +               if (global->group_index_shift >= BITS_PER_LONG ||
> +                   global->group_index_bits >
> +                   BITS_PER_LONG - global->group_index_shift) {

The above check for "too small group index shift" is fine but why
is the existing check for "too big group index shift" not sufficient.

>                         pr_err("%pfwP: group index shift too big\n", fwnode);
>                         return -EINVAL;
>                 }
>
> base-commit: 13ca1c0b7d8a3ab1e59cb3e780c1dd7441e22515
>

Regards,
Anup