Re: [PATCH v3 23/24] hw/i2c/pmbus: fix undefined behavior in pmbus_direct_mode2data

["Daniel P. Berrangé" <berrange@redhat.com>]

On Sat, May 16, 2026 at 11:59:22AM +0400, Marc-André Lureau wrote:
> The intermediate result of (Y * 10^-R - b) / m can be negative when
> the bias (b) is large and the raw register value is small (e.g. zero
> on an uninitialized device). Assigning that negative double to uint32_t
> is undefined behavior, caught by UBSan/clang.
> 
> Use a double intermediate and clamp negative results to zero.
> 
> Fixes: 3746d5c15e70 ("hw/i2c: add support for PMBus")
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  hw/i2c/pmbus_device.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
> index 853dc4b4342..861f5b4fb63 100644
> --- a/hw/i2c/pmbus_device.c
> +++ b/hw/i2c/pmbus_device.c
> @@ -23,8 +23,8 @@ uint16_t pmbus_data2direct_mode(PMBusCoefficients c, uint32_t value)
>  uint32_t pmbus_direct_mode2data(PMBusCoefficients c, uint16_t value)
>  {
>      /* X = (Y * 10^-R - b) / m */
> -    uint32_t x = (value / pow(10, c.R) - c.b) / c.m;
> -    return x;
> +    double x = (value / pow(10, c.R) - c.b) / c.m;
> +    return (x > 0) ? (uint32_t)x : 0;

Couldn't  'x' exceed  G_MAXUINT32 and thus truncate here which
while not undefined would still seem undesirable ? If so, then
perhaps

  return (x > 0
            ? (x < G_MAXUINT32 ? (uint32_t)x : G_MAXUINT32)
	    : 0);

?

>  }
>  
>  uint16_t pmbus_data2linear_mode(uint16_t value, int exp)
> 
> -- 
> 2.54.0
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|