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

From: "Daniel P. Berrange" <berrange@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Gerd Hoffmann <kraxel@redhat.com>,
	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 14:12:56 +0000	[thread overview]
Message-ID: <20180118141256.GS19695@redhat.com> (raw)
In-Reply-To: <b56b41f6-f048-00bf-d5fb-a1ed20ddeb09@redhat.com>

On Thu, Jan 18, 2018 at 02:54:48PM +0100, Paolo Bonzini wrote:
> On 18/01/2018 14:36, Daniel P. Berrange wrote:
> >>> +/*
> >>> + * 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.
> > I guess we can usefully change client_width/client_height to be an unsigned
> > int, since there's no valid scenario for them to be negative.
> 
> In addition to that, do we support a >= 2 GiB framebuffer at all? (Even
> with unsigned ints, Coverity would rightly complain about a truncated
> 32-bit multiplication being assigned to a 64-bit value).

client_width/client_height are values that are initialized from the
graphics card frontend config, and thus limited by amount of video
RAM QEMU allows.   bytes_per_pixel is limited to 8/16/32.

So I think we're safe from 2GB overflow in any normal case.

That said, VGA RAM size is configurable, so I'm curious what would happen
if someone configured an insanely large VGA RAM and asked for a big frame
buffer in guest.

VNC is protocol limited to uint16 for width/height size, and so is X11
so I imagine some exploding behavour would follow :-)

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|