[Spice-devel] [PATCH spice-server] gstreamer: Prevent integer overflow in delay computation

[Francois Gouget]

On Mon, 12 Dec 2016, Frediano Ziglio wrote:

> The partial expression "MSEC_PER_SEC * size * 8" can overflow if
> size is 536870 or more. This as the operation is done using
> 32 bit unsigned integers. Being the size potentially double of
> a compressed frame size the limit can be easily reached.
> As get_average_frame_size already return a 64 bit use 64 bit
> even for the size to avoid the integer overflow.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/gstreamer-encoder.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
> index e28ab00..988d193 100644
> --- a/server/gstreamer-encoder.c
> +++ b/server/gstreamer-encoder.c
> @@ -511,7 +511,7 @@ static uint32_t get_min_playback_delay(SpiceGstEncoder *encoder)
>       * an I frame) and an average frame. This also takes into account the
>       * frames dropped by the encoder bit rate control.
>       */
> -    uint32_t size = get_maximum_frame_size(encoder) + get_average_frame_size(encoder);
> +    uint64_t size = get_maximum_frame_size(encoder) + get_average_frame_size(encoder);
>      uint32_t send_time = MSEC_PER_SEC * size * 8 / encoder->bit_rate;
>  
>      /* Also factor in the network latency with a margin for jitter. */
> 

536870 bytes seems really large for a single frame but being prepared 
for that case makes sense anyway.

Acked-by: Francois Gouget <fgouget at codeweavers.com>

-- 
Francois Gouget <fgouget at codeweavers.com>