On Thu, Jan 18, 2018 at 01:29:35PM +0000, Peter Maydell wrote: > On 12 January 2018 at 12:58, Gerd Hoffmann <kra...@redhat.com> wrote: > > From: "Daniel P. Berrange" <berra...@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 :|