|
From: | Philippe Mathieu-Daudé |
Subject: | Re: [PATCH 1/1] hw: aspeed_gpio: Fix GPIO array indexing |
Date: | Sat, 25 Sep 2021 13:02:54 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.0 |
Hi Peter, On 9/24/21 08:19, 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 that match both the hardware layout and this existing indexing code.
This is one logical change: 1 patch
Also, I noticed that some of the property specifications looked wrong: the lower 8 bits in the input and output u32's correspond to the first group label, and the upper 8 bits correspond to the last group label.
Another logical change: another patch :) So please split this patch in 2. Maybe easier to fix GPIOSetProperties first, then convert from dense to sparse array. Regards, Phil.
I looked at the datasheet and several of these declarations seemed incorrect to me (I was basing it off of the I/O column). If somebody can double-check this, I'd really appreciate it! Some were definitely wrong though, like "Y" and "Z" in the 2600. @@ -796,7 +776,7 @@ static const GPIOSetProperties ast2500_set_props[] = { [3] = {0xffffffff, 0xffffffff, {"M", "N", "O", "P"} }, [4] = {0xffffffff, 0xffffffff, {"Q", "R", "S", "T"} }, [5] = {0xffffffff, 0x0000ffff, {"U", "V", "W", "X"} }, - [6] = {0xffffff0f, 0x0fffff0f, {"Y", "Z", "AA", "AB"} }, + [6] = {0x0fffffff, 0x0fffffff, {"Y", "Z", "AA", "AB"} }, [7] = {0x000000ff, 0x000000ff, {"AC"} }, }; @@ -805,9 +785,9 @@ static GPIOSetProperties ast2600_3_3v_set_props[] = { [1] = {0xffffffff, 0xffffffff, {"E", "F", "G", "H"} }, [2] = {0xffffffff, 0xffffffff, {"I", "J", "K", "L"} }, [3] = {0xffffffff, 0xffffffff, {"M", "N", "O", "P"} }, - [4] = {0xffffffff, 0xffffffff, {"Q", "R", "S", "T"} }, - [5] = {0xffffffff, 0x0000ffff, {"U", "V", "W", "X"} }, - [6] = {0xffff0000, 0x0fff0000, {"Y", "Z", "", ""} }, + [4] = {0xffffffff, 0x00ffffff, {"Q", "R", "S", "T"} }, + [5] = {0xffffffff, 0xffffff00, {"U", "V", "W", "X"} }, + [6] = {0x0000ffff, 0x0000ffff, {"Y", "Z", "", ""} }, }; Signed-off-by: Peter Delevoryas <pdel@fb.com> --- hw/gpio/aspeed_gpio.c | 80 +++++++++++++++-------------------- include/hw/gpio/aspeed_gpio.h | 5 +-- 2 files changed, 35 insertions(+), 50 deletions(-)
[Prev in Thread] | Current Thread | [Next in Thread] |