All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] kvm: mpic: limit active IRQ sources to NUM_OUTPUTS
@ 2019-11-15  5:18 P J P
  2019-11-20  2:33 ` Paul Mackerras
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: P J P @ 2019-11-15  5:18 UTC (permalink / raw)
  To: kvm-ppc

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;
 		pr_debug("Set ILR %d to 0x%08x, output %d\n", n_IRQ, src->idr,
 			src->output);

--
2.21.0

^ permalink raw reply related	[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
                   ` (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

end of thread, other threads:[~2019-11-27  8:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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

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.