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

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

On Thu, Jan 18, 2018 at 01:29:35PM +0000, Peter Maydell wrote:
> 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.

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.

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 :|