| Message ID | 20181026073034.16648-1-ppandit@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] strongarm: mask off high[32:28] bits from dir and state registers | expand |
On 26 October 2018 at 08:30, P J P <ppandit@redhat.com> wrote: > From: Prasad J Pandit <pjp@fedoraproject.org> > > 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. > > Reported-by: Moguofang <moguofang@huawei.com> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > --- > 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 > > diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c > index ec2627374d..dd8c4b1f2e 100644 > --- a/hw/arm/strongarm.c > +++ b/hw/arm/strongarm.c > @@ -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
On 30/10/18 12:49, P J P wrote: > +-- 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] */ > > Thank you. > -- > Prasad J Pandit / Red Hat Product Security Team > 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F >
diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c index ec2627374d..dd8c4b1f2e 100644 --- a/hw/arm/strongarm.c +++ b/hw/arm/strongarm.c @@ -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;