Re: [Qemu-devel] [PATCH for 2.10 v2 19/20] spapr_vio: fix overflow of qdevs in spapr_dt_vdevice()

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2002 bytes --]

On Wed, Jul 26, 2017 at 11:42:23PM -0300, Philippe Mathieu-Daudé wrote:
> sizeof(ptr) was used instead of sizeof(struct)...
> 
> also use g_malloc_n() which take care of possible type overflow.
> 
> hw/ppc/spapr_vio.c:641:22: warning: The code calls sizeof() on a pointer type. This can produce an unexpected result
>     qdevs = g_malloc(sizeof(qdev) * num);
>                      ^     ~~~~~~
> hw/ppc/spapr_vio.c:648:23: warning: The code calls sizeof() on a pointer type. This can produce an unexpected result
>     qsort(qdevs, num, sizeof(qdev), compare_reg);
>                       ^     ~~~~~~
> 
> Reported-by: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Nack.

Have a closer look, what's going in the array really is pointers, not
structures.  This is a false warning from clang, we need to find a
different way to suppress it.

> ---
>  hw/ppc/spapr_vio.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> index ea3bc8bd9e..9991b44c9f 100644
> --- a/hw/ppc/spapr_vio.c
> +++ b/hw/ppc/spapr_vio.c
> @@ -638,14 +638,14 @@ void spapr_dt_vdevice(VIOsPAPRBus *bus, void *fdt)
>      }
>  
>      /* Copy out into an array of pointers */
> -    qdevs = g_malloc(sizeof(qdev) * num);
> +    qdevs = g_malloc_n(num, sizeof(*qdev));
>      num = 0;
>      QTAILQ_FOREACH(kid, &bus->bus.children, sibling) {
>          qdevs[num++] = kid->child;
>      }
>  
>      /* Sort the array */
> -    qsort(qdevs, num, sizeof(qdev), compare_reg);
> +    qsort(qdevs, num, sizeof(*qdev), compare_reg);
>  
>      /* Hack alert. Give the devices to libfdt in reverse order, we happen
>       * to know that will mean they are in forward order in the tree. */

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]