Re: [PATCH] x86/APIC: handle overflow in TMICT calculation

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.04.2026 11:39, Andrew Cooper wrote:
> On 09/04/2026 10:21 am, Jan Beulich wrote:
>> With an expiry value on the order of 20 hours, and with a bus scale value
>> of 256k (as supplied by qemu), the (signed) multiplication will be UB. As
>> we've checked that the value is positive, we mean unsigned multiplication
>> anyway. Yet let's play safe against even larger expiry and bus scale
>> values, leveraging the compiler builtin that there is for this purpose.
>>
>> While there also drop the stray cast from the actual TMICT write.
>>
>> Fixes: 9062553a0dc1 ("added time and accurate timer support")
>> Fixes: b95beb185810 ("x86: Clean up APIC local timer handling")
>> Reported-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Thanks.

>> --- a/xen/arch/x86/apic.c
>> +++ b/xen/arch/x86/apic.c
>> @@ -1224,10 +1224,16 @@ int reprogram_timer(s_time_t timeout)
>>      }
>>  
>>      if ( timeout && ((expire = timeout - NOW()) > 0) )
>> -        apic_tmict = min_t(uint64_t, (bus_scale * expire) >> 
>> BUS_SCALE_SHIFT,
>> -                           UINT32_MAX);
>> +    {
>> +        unsigned long product;
>>  
>> -    apic_write(APIC_TMICT, (unsigned long)apic_tmict);
>> +        apic_tmict = UINT32_MAX;
>> +        if ( !__builtin_umull_overflow(bus_scale, expire, &product) &&
>> +             (product >>= BUS_SCALE_SHIFT) < apic_tmict )
>> +            apic_tmict = product;
>> +    }
>> +
>> +    apic_write(APIC_TMICT, apic_tmict);
>>  
>>      return apic_tmict || !timeout;
>>  }
> 
> This is fine for staging, but be aware it cannot be backported before
> 4.21 due to the toolchain baseline (and nothing in CI will notice, I
> don't think.)

I'm debating with myself whether to replace by an asm() there. (If we expected
further uses of those overflow built-ins, we could consider adding non-built-
in fallbacks in those older branches. Yet unless something like this was needed
in an XSA, it would be solely 4.20 to gain such.)

Luckily in this case I think I would notice myself, as by default I'm building
the older trees with gcc 4.8.5 and 7.4.

Jan