Patchwork fix Coverity scan SIGN_EXTENSION error

login
register
mail settings
Submitter Gerd Hoffmann
Date June 6, 2013, 3:12 p.m.
Message ID <1370531573-17974-1-git-send-email-kraxel@redhat.com>
Download mbox | patch
Permalink /patch/249451/
State New
Headers show

Comments

Gerd Hoffmann - June 6, 2013, 3:12 p.m.
Cc: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/qxl-render.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Laszlo Ersek - June 6, 2013, 6:52 p.m.
On 06/06/13 17:12, Gerd Hoffmann wrote:
> Cc: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/display/qxl-render.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
> index f511a62..269b1a7 100644
> --- a/hw/display/qxl-render.c
> +++ b/hw/display/qxl-render.c
> @@ -199,7 +199,7 @@ static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor)
>      c->hot_y = cursor->header.hot_spot_y;
>      switch (cursor->header.type) {
>      case SPICE_CURSOR_TYPE_ALPHA:
> -        size = cursor->header.width * cursor->header.height * sizeof(uint32_t);
> +        size = sizeof(uint32_t) * cursor->header.width * cursor->header.height;
>          memcpy(c->data, cursor->chunk.data, size);
>          if (qxl->debug > 2) {
>              cursor_print_ascii_art(c, "qxl/alpha");
> 

Right. "width" and "height" are both "uint16_t" (short unsigned) and
they are promoted to "int". The binding is then

  (int * int) * size_t
  int         * size_t
  size_t      * size_t
  size_t

By the time the product would be converted to size_t ("unsigned" or
"unsigned long" in practice, dependent on ILP32 or LP64), it may have
overflowed (undefined behavior for signed int).

The above changes it to

  (size_t * int)    * int
  (size_t * size_t) * int
  size_t            * int
  size_t            * size_t
  size_t

(BTW this is not order of evaluation, just how the types are determined
by operator binding.)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Andreas Färber - June 6, 2013, 10:18 p.m.
Am 06.06.2013 17:12, schrieb Gerd Hoffmann:
> Cc: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Please indicate in the subject that this is about qxl when queuing. :)

Cheers,
Andreas

> ---
>  hw/display/qxl-render.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
> index f511a62..269b1a7 100644
> --- a/hw/display/qxl-render.c
> +++ b/hw/display/qxl-render.c
> @@ -199,7 +199,7 @@ static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor)
>      c->hot_y = cursor->header.hot_spot_y;
>      switch (cursor->header.type) {
>      case SPICE_CURSOR_TYPE_ALPHA:
> -        size = cursor->header.width * cursor->header.height * sizeof(uint32_t);
> +        size = sizeof(uint32_t) * cursor->header.width * cursor->header.height;
>          memcpy(c->data, cursor->chunk.data, size);
>          if (qxl->debug > 2) {
>              cursor_print_ascii_art(c, "qxl/alpha");

Patch

diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index f511a62..269b1a7 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -199,7 +199,7 @@  static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor)
     c->hot_y = cursor->header.hot_spot_y;
     switch (cursor->header.type) {
     case SPICE_CURSOR_TYPE_ALPHA:
-        size = cursor->header.width * cursor->header.height * sizeof(uint32_t);
+        size = sizeof(uint32_t) * cursor->header.width * cursor->header.height;
         memcpy(c->data, cursor->chunk.data, size);
         if (qxl->debug > 2) {
             cursor_print_ascii_art(c, "qxl/alpha");