[Qemu-devel] [PATCH for-4.0 0/9] ppc: get rid of g_malloc(sizeof(T) * n)

Greg Kurz posted 9 patches 5 years, 4 months ago
Test asan passed
Test checkpatch passed
Test docker-quick@centos7 passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/154332389387.541746.8099441653585015043.stgit@bahia.lab.toulouse-stg.fr.ibm.com
hw/ppc/e500.c                   |   18 ++++++++----------
hw/ppc/mac_newworld.c           |   30 +++++++++++++-----------------
hw/ppc/ppc405_boards.c          |    4 ++--
hw/ppc/ppc405_uc.c              |    4 ++--
hw/ppc/ppc440_bamboo.c          |    5 ++---
hw/ppc/sam460ex.c               |    2 +-
hw/ppc/spapr_iommu.c            |    2 +-
hw/ppc/spapr_vio.c              |    2 +-
hw/ppc/virtex_ml507.c           |    2 +-
include/hw/ppc/openpic.h        |    2 ++
target/ppc/translate_init.inc.c |    6 +++---
11 files changed, 36 insertions(+), 41 deletions(-)
[Qemu-devel] [PATCH for-4.0 0/9] ppc: get rid of g_malloc(sizeof(T) * n)
Posted by Greg Kurz 5 years, 4 months ago
As explained in HACKING, the g_malloc(sizeof(T) * n) construct is unsafe
because it can't detect multiplication overflowing size_t and doesn't
allow type checking.

It appears to be used in a bunch of places though:

$ git grep -E 'malloc.*sizeof' | grep ' \* '  | wc -l
101

This series fixes the ppc target and ppc machine code. The changes are
mostly trivial. Only the mac99 and e500 machines required some more work
that should be reviewed carefully, as it was only compile-tested.

--
Greg

---

Greg Kurz (9):
      target/ppc: use g_new(T, n) instead of g_malloc(sizeof(T) * n)
      spapr: use g_new(T, n) instead of g_malloc(sizeof(T) * n)
      ppc405_boards: use g_new(T, n) instead of g_malloc(sizeof(T) * n)
      ppc405_uc: use g_new(T, n) instead of g_malloc(sizeof(T) * n)
      ppc440_bamboo: use g_new(T, n) instead of g_malloc(sizeof(T) * n)
      sam460ex: use g_new(T, n) instead of g_malloc(sizeof(T) * n)
      virtex_ml507: use g_new(T, n) instead of g_malloc(sizeof(T) * n)
      mac_newworld: simplify IRQ wiring
      e500: simplify IRQ wiring


 hw/ppc/e500.c                   |   18 ++++++++----------
 hw/ppc/mac_newworld.c           |   30 +++++++++++++-----------------
 hw/ppc/ppc405_boards.c          |    4 ++--
 hw/ppc/ppc405_uc.c              |    4 ++--
 hw/ppc/ppc440_bamboo.c          |    5 ++---
 hw/ppc/sam460ex.c               |    2 +-
 hw/ppc/spapr_iommu.c            |    2 +-
 hw/ppc/spapr_vio.c              |    2 +-
 hw/ppc/virtex_ml507.c           |    2 +-
 include/hw/ppc/openpic.h        |    2 ++
 target/ppc/translate_init.inc.c |    6 +++---
 11 files changed, 36 insertions(+), 41 deletions(-)


Re: [Qemu-devel] [PATCH for-4.0 0/9] ppc: get rid of g_malloc(sizeof(T) * n)
Posted by Eric Blake 5 years, 4 months ago
On 11/27/18 7:04 AM, Greg Kurz wrote:
> As explained in HACKING, the g_malloc(sizeof(T) * n) construct is unsafe
> because it can't detect multiplication overflowing size_t and doesn't
> allow type checking.
> 
> It appears to be used in a bunch of places though:
> 
> $ git grep -E 'malloc.*sizeof' | grep ' \* '  | wc -l
> 101
> 
> This series fixes the ppc target and ppc machine code. The changes are
> mostly trivial. Only the mac99 and e500 machines required some more work
> that should be reviewed carefully, as it was only compile-tested.

Did you do this all manually, or did you try to use Coccinelle?  Hmm - 
we have a Coccinelle script for this mentioned in commit b45c03f (most 
recently reused in bdd81add) - but it is not yet in scripts/coccinelle/. 
  Maybe that would be worth doing now.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH for-4.0 0/9] ppc: get rid of g_malloc(sizeof(T) * n)
Posted by Greg Kurz 5 years, 4 months ago
On Tue, 27 Nov 2018 07:16:44 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 11/27/18 7:04 AM, Greg Kurz wrote:
> > As explained in HACKING, the g_malloc(sizeof(T) * n) construct is unsafe
> > because it can't detect multiplication overflowing size_t and doesn't
> > allow type checking.
> > 
> > It appears to be used in a bunch of places though:
> > 
> > $ git grep -E 'malloc.*sizeof' | grep ' \* '  | wc -l
> > 101
> > 
> > This series fixes the ppc target and ppc machine code. The changes are
> > mostly trivial. Only the mac99 and e500 machines required some more work
> > that should be reviewed carefully, as it was only compile-tested.  
> 
> Did you do this all manually, or did you try to use Coccinelle?  Hmm - 
> we have a Coccinelle script for this mentioned in commit b45c03f (most 
> recently reused in bdd81add) - but it is not yet in scripts/coccinelle/. 
>   Maybe that would be worth doing now.
> 

I did that manually because I didn't know about Markus's Coccinelle
script... Also, I've only fixed the case involving a multiplication,
since HACKING says "g_malloc(sizeof(*v)) are acceptable".

I'll have a look at adding the script in scripts/coccinelle/.

Cheers,

--
Greg

Re: [Qemu-devel] [PATCH for-4.0 0/9] ppc: get rid of g_malloc(sizeof(T) * n)
Posted by David Gibson 5 years, 4 months ago
On Tue, Nov 27, 2018 at 02:04:53PM +0100, Greg Kurz wrote:
> As explained in HACKING, the g_malloc(sizeof(T) * n) construct is unsafe
> because it can't detect multiplication overflowing size_t and doesn't
> allow type checking.
> 
> It appears to be used in a bunch of places though:
> 
> $ git grep -E 'malloc.*sizeof' | grep ' \* '  | wc -l
> 101
> 
> This series fixes the ppc target and ppc machine code. The changes are
> mostly trivial. Only the mac99 and e500 machines required some more work
> that should be reviewed carefully, as it was only compile-tested.

Series applied, thanks.


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