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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/20/2017 11:45 AM, Frediano Ziglio wrote:

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@xxxxxxxxxx>
---
   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


I did not try it.
I'll give it a try next time I'm running coverity.

I see that many other calls to memslot_get_virt are
passing sizeof(something) as size parameter so basically
also silently converting to 32 bit.

An overflow would still be possible with a uint32_t size;
I do not know if coverity checks that too or not.

Thanks,
    Uri.

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]