* Re: [PATCH v2] kvm: mpic: limit active IRQ sources to NUM_OUTPUTS
2019-11-15 5:18 [PATCH v2] kvm: mpic: limit active IRQ sources to NUM_OUTPUTS P J P
@ 2019-11-20 2:33 ` Paul Mackerras
2019-11-20 11:12 ` P J P
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Paul Mackerras @ 2019-11-20 2:33 UTC (permalink / raw)
To: kvm-ppc
On Fri, Nov 15, 2019 at 10:36:20AM +0530, P J P wrote:
> From: P J P <pjp@fedoraproject.org>
>
> openpic_src_write sets interrupt level 'src->output' masked with
> ILR_INTTGT_MASK(=0xFF). It's then used to index 'dst->outputs_active'
> array. With NUM_OUTPUTS=3, it may lead to OOB array access. Limit
> active IRQ sources to < NUM_OUTPUTS.
>
> Reported-by: Reno Robert <renorobert@gmail.com>
> Signed-off-by: P J P <pjp@fedoraproject.org>
> ---
> arch/powerpc/kvm/mpic.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Update v2: limit IRQ sources to NUM_OUTPUTS
> -> https://www.spinics.net/lists/kvm-ppc/msg16554.html
>
> diff --git a/arch/powerpc/kvm/mpic.c b/arch/powerpc/kvm/mpic.c
> index fe312c160d97..fe4afd54c6e7 100644
> --- a/arch/powerpc/kvm/mpic.c
> +++ b/arch/powerpc/kvm/mpic.c
> @@ -628,7 +628,7 @@ static inline void write_IRQreg_ilr(struct openpic *opp, int n_IRQ,
> if (opp->flags & OPENPIC_FLAG_ILR) {
> struct irq_source *src = &opp->src[n_IRQ];
>
> - src->output = val & ILR_INTTGT_MASK;
> + src->output = val % NUM_OUTPUTS;
Still not right, I'm afraid, since it could leave src->output set to
3, which would lead to an out-of-bounds array access. I think it
needs to be
if (val < NUM_OUTPUTS)
src->output = val;
else
src->output = ILR_INTTGT_INT;
or something like that.
Paul.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] kvm: mpic: limit active IRQ sources to NUM_OUTPUTS
2019-11-15 5:18 [PATCH v2] kvm: mpic: limit active IRQ sources to NUM_OUTPUTS P J P
2019-11-20 2:33 ` Paul Mackerras
@ 2019-11-20 11:12 ` P J P
2019-11-20 21:41 ` Paul Mackerras
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: P J P @ 2019-11-20 11:12 UTC (permalink / raw)
To: kvm-ppc
+-- On Wed, 20 Nov 2019, Paul Mackerras wrote --+
| Still not right, I'm afraid, since it could leave src->output set to
| 3, which would lead to an out-of-bounds array access. I think it
| needs to be
===
#include <stdio.h>
int
main (int argc, char *argv[])
{
int i = 0;
for (i = 0; i < 1024; i++) {
printf ("%d: %d\n", i, i % 0x3);
}
return 0;
}
-> https://paste.centos.org/view/fb14b3cf
===
It does not seem to set it to 0x3. When a no is divisible by 0x3,
'src->output' will be set to zero(0).
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] kvm: mpic: limit active IRQ sources to NUM_OUTPUTS
2019-11-15 5:18 [PATCH v2] kvm: mpic: limit active IRQ sources to NUM_OUTPUTS P J P
2019-11-20 2:33 ` Paul Mackerras
2019-11-20 11:12 ` P J P
@ 2019-11-20 21:41 ` Paul Mackerras
2019-11-21 4:57 ` P J P
2019-11-27 8:36 ` P J P
4 siblings, 0 replies; 6+ messages in thread
From: Paul Mackerras @ 2019-11-20 21:41 UTC (permalink / raw)
To: kvm-ppc
On Wed, Nov 20, 2019 at 04:30:32PM +0530, P J P wrote:
> +-- On Wed, 20 Nov 2019, Paul Mackerras wrote --+
> | Still not right, I'm afraid, since it could leave src->output set to
> | 3, which would lead to an out-of-bounds array access. I think it
> | needs to be
>
> => #include <stdio.h>
>
> int
> main (int argc, char *argv[])
> {
> int i = 0;
>
> for (i = 0; i < 1024; i++) {
> printf ("%d: %d\n", i, i % 0x3);
> }
>
> return 0;
> }
>
> -> https://paste.centos.org/view/fb14b3cf
> =>
> It does not seem to set it to 0x3. When a no is divisible by 0x3,
> 'src->output' will be set to zero(0).
You're right, I misread the '%' as '&'.
Paul.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] kvm: mpic: limit active IRQ sources to NUM_OUTPUTS
2019-11-15 5:18 [PATCH v2] kvm: mpic: limit active IRQ sources to NUM_OUTPUTS P J P
` (2 preceding siblings ...)
2019-11-20 21:41 ` Paul Mackerras
@ 2019-11-21 4:57 ` P J P
2019-11-27 8:36 ` P J P
4 siblings, 0 replies; 6+ messages in thread
From: P J P @ 2019-11-21 4:57 UTC (permalink / raw)
To: kvm-ppc
Hello Paul,
+-- On Thu, 21 Nov 2019, Paul Mackerras wrote --+
| > It does not seem to set it to 0x3. When a no is divisible by 0x3,
| > 'src->output' will be set to zero(0).
|
| You're right, I misread the '%' as '&'.
Considering E500 is mostly used for SoC/Embedded systems. It is not clear if
this issue can be misused from a guest running on PPC E500 platform.
...wdyt?
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] kvm: mpic: limit active IRQ sources to NUM_OUTPUTS
2019-11-15 5:18 [PATCH v2] kvm: mpic: limit active IRQ sources to NUM_OUTPUTS P J P
` (3 preceding siblings ...)
2019-11-21 4:57 ` P J P
@ 2019-11-27 8:36 ` P J P
4 siblings, 0 replies; 6+ messages in thread
From: P J P @ 2019-11-27 8:36 UTC (permalink / raw)
To: kvm-ppc
+-- On Thu, 21 Nov 2019, P J P wrote --+
| Considering E500 is mostly used for SoC/Embedded systems. It is not clear if
| this issue can be misused from a guest running on PPC E500 platform.
|
| ...wdyt?
Paul, any idea?
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
^ permalink raw reply [flat|nested] 6+ messages in thread