* [Qemu-devel] [PATCH] display: check irq handler index before access
@ 2017-12-11 10:55 P J P
2017-12-11 11:11 ` Peter Maydell
0 siblings, 1 reply; 3+ messages in thread
From: P J P @ 2017-12-11 10:55 UTC (permalink / raw)
To: Qemu Developers; +Cc: Peter Maydell, Moguofang, qemu-arm, Prasad J Pandit
From: Prasad J Pandit <pjp@fedoraproject.org>
The ctz32() routine could return value greater than
TC6393XB_GPIOS=16. This could lead to an OOB array access.
Add check to avoid it.
Reported-by: Moguofang <moguofang@huawei.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
hw/display/tc6393xb.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/hw/display/tc6393xb.c b/hw/display/tc6393xb.c
index 74d10af3d4..78292cb847 100644
--- a/hw/display/tc6393xb.c
+++ b/hw/display/tc6393xb.c
@@ -175,6 +175,10 @@ static void tc6393xb_gpio_handler_update(TC6393xbState *s)
for (diff = s->prev_level ^ level; diff; diff ^= 1 << bit) {
bit = ctz32(diff);
+ if (bit >= TC6393XB_GPIOS) {
+ fprintf(stderr, "TC6393xb: no GPIO pin %d\n", bit);
+ return;
+ }
qemu_set_irq(s->handler[bit], (level >> bit) & 1);
}
--
2.13.6
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] display: check irq handler index before access
2017-12-11 10:55 [Qemu-devel] [PATCH] display: check irq handler index before access P J P
@ 2017-12-11 11:11 ` Peter Maydell
2017-12-12 4:42 ` P J P
0 siblings, 1 reply; 3+ messages in thread
From: Peter Maydell @ 2017-12-11 11:11 UTC (permalink / raw)
To: P J P; +Cc: Qemu Developers, Moguofang, qemu-arm, Prasad J Pandit
On 11 December 2017 at 10:55, P J P <ppandit@redhat.com> wrote:
> From: Prasad J Pandit <pjp@fedoraproject.org>
>
> The ctz32() routine could return value greater than
> TC6393XB_GPIOS=16. This could lead to an OOB array access.
> Add check to avoid it.
>
> Reported-by: Moguofang <moguofang@huawei.com>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
> hw/display/tc6393xb.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/hw/display/tc6393xb.c b/hw/display/tc6393xb.c
> index 74d10af3d4..78292cb847 100644
> --- a/hw/display/tc6393xb.c
> +++ b/hw/display/tc6393xb.c
> @@ -175,6 +175,10 @@ static void tc6393xb_gpio_handler_update(TC6393xbState *s)
>
> for (diff = s->prev_level ^ level; diff; diff ^= 1 << bit) {
> bit = ctz32(diff);
> + if (bit >= TC6393XB_GPIOS) {
> + fprintf(stderr, "TC6393xb: no GPIO pin %d\n", bit);
> + return;
> + }
> qemu_set_irq(s->handler[bit], (level >> bit) & 1);
> }
It would be more sensible to just mask off the top bits of
'level' before starting the loop, rather than checking every
time around the loop:
level &= MAKE_64BIT_MASK(0, TC6493XB_GPIOS);
Also, if we want to warn about the guest having set a GPIO
bit we don't implement this isn't the best place to do it --
it would make more sense to do that only if the guest writes
something non-zero to SCR_GPO_DSR(2), and as an LOG_UNIMP warning.
But I don't think it's worth the effort since this is only used
on the ancient 'tosa' board.
thanks
-- PMM
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] display: check irq handler index before access
2017-12-11 11:11 ` Peter Maydell
@ 2017-12-12 4:42 ` P J P
0 siblings, 0 replies; 3+ messages in thread
From: P J P @ 2017-12-12 4:42 UTC (permalink / raw)
To: Peter Maydell; +Cc: Qemu Developers, Moguofang, qemu-arm
+-- On Mon, 11 Dec 2017, Peter Maydell wrote --+
| It would be more sensible to just mask off the top bits of
| 'level' before starting the loop, rather than checking every
| time around the loop:
| level &= MAKE_64BIT_MASK(0, TC6493XB_GPIOS);
Sent a revised patch v1. Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-12-12 4:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-11 10:55 [Qemu-devel] [PATCH] display: check irq handler index before access P J P
2017-12-11 11:11 ` Peter Maydell
2017-12-12 4:42 ` P J P
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.