Re: Regression on Macchiatobin from the irqchip driver

[Thomas Gleixner <tglx@linutronix.de>]

On Wed, Aug 21 2024 at 16:50, Maxime Chevallier wrote:
> By looking at the msi_lib_irq_domain_select() implementation however, I
> notice that it appears to be expected that these ops can be NULL by
> looking at the check in the return line :
>
> 	return ops && !!(ops->bus_select_mask & busmask);
>
> However, the line above dereferences the ops pointer without prior
> check :
>
> 	/* Handle pure domain searches */
> 	if (bus_token == ops->bus_select_token)
> 		return 1;

Oops.

> As I said, this area of the kernel isn't very familiar to me, but I got
> my board to boot with the following patch :
>
> --- a/drivers/irqchip/irq-msi-lib.c
> +++ b/drivers/irqchip/irq-msi-lib.c
> @@ -128,6 +128,9 @@ int msi_lib_irq_domain_select(struct irq_domain *d, struct irq_fwspec *fwspec,
>         const struct msi_parent_ops *ops = d->msi_parent_ops;
>         u32 busmask = BIT(bus_token);
>  
> +       if (!ops)
> +               return 0;
> +
>         if (fwspec->fwnode != d->fwnode || fwspec->param_count != 0)
>                 return 0;
>  
> @@ -135,6 +138,6 @@ int msi_lib_irq_domain_select(struct irq_domain *d, struct irq_fwspec *fwspec,
>         if (bus_token == ops->bus_select_token)
>                 return 1;
>  
> -       return ops && !!(ops->bus_select_mask & busmask);
> +       return !!(ops->bus_select_mask & busmask);
>
> ----------------------------
>
> I have zero confidence that this is the correct solution to the issue
> so feel free to ditch that solution :) I'll gladly test any
> patch for that on the MCBIN.

It obviously is the proper solution check after use is pretty pointless
as you demonstrated. Care to send a proper patch?

Thanks,

        tglx