Re: [PATCH 1/1] hw: aspeed_gpio: Fix GPIO array indexing

[Cédric Le Goater]

On 10/4/21 11:07, Cédric Le Goater wrote:
> On 9/28/21 05:43, pdel@fb.com wrote:
> > From: Peter Delevoryas <pdel@fb.com>
> >
> > The gpio array is declared as a dense array:
> >
> >    qemu_irq gpios[ASPEED_GPIO_NR_PINS];
> >
> > (AST2500 has 228, AST2400 has 216, AST2600 has 208)
> >
> > However, this array is used like a matrix of GPIO sets
> > (e.g. gpio[NR_SETS][NR_PINS_PER_SET] = gpio[8][32])
> >
> >    size_t offset = set * GPIOS_PER_SET + gpio;
> >    qemu_set_irq(s->gpios[offset], !!(new & mask));
> >
> > This can result in an out-of-bounds access to "s->gpios" because the
> > gpio sets do _not_ have the same length. Some of the groups (e.g.
> > GPIOAB) only have 4 pins. 228 != 8 * 32 == 256.
> >
> > To fix this, I converted the gpio array from dense to sparse, to match
> > both the hardware layout and this existing indexing code.
> >
> > Fixes: 4b7f956862dc2db4c5c ("hw/gpio: Add basic Aspeed GPIO model for AST2400 and 
> > AST2500")
> > Signed-off-by: Peter Delevoryas <pdel@fb.com>
>
>
> This patch is raising an error in qtest-arm/qom-test when compiled
> with clang :
>
> Running test qtest-arm/qom-test
> Unexpected error in object_property_try_add() at ../qom/object.c:1224:
> qemu-system-arm: attempt to add duplicate property 'gpio0' to object (type 
> 'aspeed.gpio-ast2600-1_8v')
> Broken pipe
> ERROR qtest-arm/qom-test - too few tests run (expected 78, got 0)
> make: *** [Makefile.mtest:24: run-test-1] Error 1

The GPIOSetProperties arrary is smaller for the ast2600_1_8v model :

  static GPIOSetProperties ast2600_1_8v_set_props[] = {
      [0] = {0xffffffff,  0xffffffff,  {"18A", "18B", "18C", "18D"} },
      [1] = {0x0000000f,  0x0000000f,  {"18E"} },
  };

and in aspeed_gpio_init() :

    for (int i = 0; i < ASPEED_GPIO_MAX_NR_SETS; i++) {

we loop beyond.

To configure compilation with clang, use the configure option :

  --cc=clang

and simply run 'make check-qtest-arm'

Thanks

C.