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

["Cédric Le Goater" <clg@kaod.org>]

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.