Re: [Qemu-devel] [PULL 10/14] ui: fix VNC client throttling when audio capture is active

From: Peter Maydell <peter.maydell@linaro.org>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: QEMU Developers <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [PULL 10/14] ui: fix VNC client throttling when audio capture is active
Date: Thu, 18 Jan 2018 13:29:35 +0000	[thread overview]
Message-ID: <CAFEAcA8UAbOvZuhxwjD2zAA7GBGucD8MAf5E0h05tb3NXKdGqw@mail.gmail.com> (raw)
In-Reply-To: <20180112125854.18261-11-kraxel@redhat.com>

On 12 January 2018 at 12:58, Gerd Hoffmann <kraxel@redhat.com> wrote:
> From: "Daniel P. Berrange" <berrange@redhat.com>
>
> The VNC server must throttle data sent to the client to prevent the 'output'
> buffer size growing without bound, if the client stops reading data off the
> socket (either maliciously or due to stalled/slow network connection).

Hi. Coverity (CID 1385147) complains about a suspicious sign extension
in this patch:

> +/*
> + * Figure out how much pending data we should allow in the output
> + * buffer before we throttle incremental display updates, and/or
> + * drop audio samples.
> + *
> + * We allow for equiv of 1 full display's worth of FB updates,
> + * and 1 second of audio samples. If audio backlog was larger
> + * than that the client would already suffering awful audio
> + * glitches, so dropping samples is no worse really).
> + */
> +static void vnc_update_throttle_offset(VncState *vs)
> +{
> +    size_t offset =
> +        vs->client_width * vs->client_height * vs->client_pf.bytes_per_pixel;

because the multiply is done with the "int" type, and then may
be sign-extended when converted to the probably-64-bit unsigned
size_t, resulting in the high bits all being set if the
multiply ended up with a 1 in bit 31.

thanks
-- PMM