[Spice-devel] [spice-server PATCH 1/3] red_get_surface_cmd: avoid overflow

[Frediano Ziglio]

> 
> On 07/17/2017 11:22 AM, Frediano Ziglio wrote:
> >>
> >> Although unlikely, theoretically, multiplying two 32-bit
> >> numbers may overflow.
> >>
> >> Found by coverity.
> >>
> >> Signed-off-by: Uri Lublin <uril at redhat.com>
> >> ---
> >>   server/red-parse-qxl.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
> >> index 33f36923a..0ffa5f7d4 100644
> >> --- a/server/red-parse-qxl.c
> >> +++ b/server/red-parse-qxl.c
> >> @@ -1397,7 +1397,7 @@ bool red_get_surface_cmd(RedMemSlotInfo *slots, int
> >> group_id,
> >>               return false;
> >>           }
> >>   
> >> -        size = red->u.surface_create.height *
> >> abs(red->u.surface_create.stride);
> >> +        size = red->u.surface_create.height *
> >> (uint64_t)abs(red->u.surface_create.stride);
> >>           red->u.surface_create.data =
> >>               (uint8_t*)memslot_get_virt(slots,
> >>               qxl->u.surface_create.data,
> >>               size, group_id, &error);
> >>           if (error) {
> > 
> > This overflow is already handled by red_validate_surface call.
> 
> Indeed.
> I'll drop this patch.
> 
> 
> > Also note that silently the uint64_t size is converted to 32 bit calling
> > memslot_get_virt so maybe would be better to change size to uint32_t.
> > 
> > Frediano
> > 
> 
> Thanks,
>      Uri.
> 
> 

Have you try to change the variable to uint32_t ?
This should make everything happy (coverity and code).

Frediano