Re: [PATCH v2] x86/apic: Avoid infinite loop in io_apic_level_ack_pending()

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 10/13/25 11:11 PM, Jason Andryuk wrote:

> io_apic_level_ack_pending() will end up in an infinite loop if
> entry->pin == -1.  entry does not change, so it will keep reading -1.
>
> Convert to a proper for loop so that continue works.  Add a new helper,
> next_entry(), to handle advancing to the next irq_pin_list entry.
>
> Fixes: f821102450a1 ("x86: IRQ Migration logic enhancement.")
> Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
>     
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>

Thanks.

~ Oleksii
> ---
> v2:
> continue (not break) for pin == -1.
>
> I added the next_entry() helper since putting the _expression_ in the for
> loop is a little cluttered.  The helper can also be re-used for other
> instances within the file.
> ---
>  xen/arch/x86/io_apic.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
> index c384f10c1b..7b58345c96 100644
> --- a/xen/arch/x86/io_apic.c
> +++ b/xen/arch/x86/io_apic.c
> @@ -1586,14 +1586,21 @@ static int __init cf_check setup_ioapic_ack(const char *s)
>  }
>  custom_param("ioapic_ack", setup_ioapic_ack);
>  
> +static struct irq_pin_list *next_entry(struct irq_pin_list *entry)
> +{
> +    if ( !entry->next )
> +        return NULL;
> +
> +    return irq_2_pin + entry->next;
> +}
> +
>  static bool io_apic_level_ack_pending(unsigned int irq)
>  {
>      struct irq_pin_list *entry;
>      unsigned long flags;
>  
>      spin_lock_irqsave(&ioapic_lock, flags);
> -    entry = &irq_2_pin[irq];
> -    for (;;) {
> +    for ( entry = &irq_2_pin[irq]; entry ; entry = next_entry(entry) ) {
>          unsigned int reg;
>          int pin;
>  
> @@ -1609,9 +1616,6 @@ static bool io_apic_level_ack_pending(unsigned int irq)
>              spin_unlock_irqrestore(&ioapic_lock, flags);
>              return 1;
>          }
> -        if (!entry->next)
> -            break;
> -        entry = irq_2_pin + entry->next;
>      }
>      spin_unlock_irqrestore(&ioapic_lock, flags);
>  
>