Re: [PATCH] hw/i2c/pmbus_device: Fix a possible crash when requesting too many bytes

On Tue, 9 Jun 2026 at 11:56, Thomas Huth <[email protected]> wrote:
>
> From: Thomas Huth <[email protected]>
>
> The pmbus_send_string() function contains an assert() statement that
> can be triggered by the guest code when requesting too many data
> without reading from the device in between. This should not be possible.
> pmbus_send() already has a similar logic, but it simply ignores the
> error after logging a message with qemu_log_mask(), so do the same now
> in pmbus_send_string() to fix the issue.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3388
> Signed-off-by: Thomas Huth <[email protected]>
> ---
>  hw/i2c/pmbus_device.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
> index b1f9843f52e..6aa608a2998 100644
> --- a/hw/i2c/pmbus_device.c
> +++ b/hw/i2c/pmbus_device.c
> @@ -104,7 +104,12 @@ void pmbus_send_string(PMBusDevice *pmdev, const char *data)
>      }
>
>      size_t len = strlen(data);
> -    g_assert(len + pmdev->out_buf_len < SMBUS_DATA_MAX_LEN);
> +    if (len + pmdev->out_buf_len >= SMBUS_DATA_MAX_LEN) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: requested too much data from PMBus device\n",
> +                      __func__);
> +        return;
> +    }

I had a look at this bug earlier, but was not sure that this is the
right thing. What happens is that the guest does something that
causes the device to queue up data X ready for the guest to
read, but then instead of reading it, the guest does another
"I would like data X please" action. My guess is that the way
the hardware handles this is probably not "add the second
copy of data X after the first one". Perhaps it is "drop the
data the guest didn't read, so the next guest read gets the
second lot of data, not the first". But maybe the spec really
does say "you can do things in the order 'ask for A, ask for
B, read data for A, read data for B".

I couldn't conveniently find the pmbus spec to find out what
the hardware is supposed to do here. We need some input from
somebody who knows about pmbus.

thanks
-- PMM