strongarm: mask off high[32:28] bits from dir and state registers
PATCH v2 1/1 strongarm: mask off high[32:28] bits from dir and state registers
From: Prasad J Pandit <[email protected]>
The high[32:28] bits of 'direction' and 'state' registers of SA-1100/SA-1110 device are reserved. Setting them may lead to OOB 's->handler[]' array access issue. Mask off [32:28] bits to avoid it.
--- hw/arm/strongarm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Update v2: mask off high[32:28] bits
-> https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg05746.html
hw/arm/strongarm.c +2 -2
@@ -587,12 +587,12 @@ static void strongarm_gpio_write(void *opaque, hwaddr offset,switch (offset) {case GPDR :/* GPIO Pin-Direction registers */ - s->dir = value; + s->dir = value &0x3fffff ;strongarm_gpio_handler_update (s);break ;case GPSR :/* GPIO Pin-Output Set registers */ - s->olevel |= value; + s->olevel |= value &0x3fffff ;strongarm_gpio_handler_update (s);break ;
Signature · 1 line
2.17.2
Quoted 6 lines · P J P
From: Prasad J Pandit <[email protected]>
The high[32:28] bits of 'direction' and 'state' registers of SA-1100/SA-1110 device are reserved. Setting them may lead to OOB 's->handler[]' array access issue. Mask off [32:28] bits to avoid it.
There is no bit 32 in a 32-bit value; you mean 31.
Quoted 15 lines
Reported-by Moguofang <[email protected]>Signed-off-by Prasad J Pandit <[email protected]>--- hw/arm/strongarm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)Update v2: mask off high[32:28] bits
-> https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg05746.htmlhw/arm/strongarm.c +0
-0
hw/arm/strongarm.c static void strongarm_gpio_write(void *opaque, hwaddr offset, · line 590
Quoted 5 lines
hw/arm/strongarm.c +1
-1@@ -587,12 +587,12 @@ static void strongarm_gpio_write(void *opaque, hwaddr offset,switch (offset) {case GPDR :/* GPIO Pin-Direction registers */ - s->dir = value; + s->dir = value &0x3fffff ;
The commit message says it's masking [31:28], but the code is masking [31:22]. The SA1110 spec suggests the commit message is correct and the code is not.
thanks
-- PMM
+-- On Mon, 29 Oct 2018, Peter Maydell wrote --+
| > switch (offset) {
| > case GPDR: /* GPIO Pin-Direction registers */
| > - s->dir = value;
| > + s->dir = value & 0x3fffff;
|
| The commit message says it's masking [31:28], but the
| code is masking [31:22]. The SA1110 spec suggests the
| commit message is correct and the code is not.Ouch, sorry! Sent revised patch v3.
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
Quoted 11 lines · P J P
+-- On Mon, 29 Oct 2018, Peter Maydell wrote --+ | > switch (offset) { | > case GPDR: /* GPIO Pin-Direction registers */ | > - s->dir = value; | > + s->dir = value & 0x3fffff; | | The commit message says it's masking [31:28], but the | code is masking [31:22]. The SA1110 spec suggests the | commit message is correct and the code is not.Ouch, sorry! Sent revised patch v3.
That's where the extract32() is more convenient and less bug prone:
s->dir = extract32(value, 0, 28); /* mask off [31:28] */
Quoted 6 lines
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F