* [PATCH] KVM: arm64: fix potential bug
@ 2019-03-12 0:32 John Gong
2019-03-12 1:42 ` André Przywara
0 siblings, 1 reply; 4+ messages in thread
From: John Gong @ 2019-03-12 0:32 UTC (permalink / raw)
To: christoffer.dall
Cc: Shengmin Gong, Marc Zyngier, Julien Thierry, Jia He,
Andre Przywara, linux-arm-kernel, kvmarm, linux-kernel
Since intid always >= VGIC_NR_PRIVATE_IRQS, so then even vcpu == NULL,
it never return -EINVAL.
Signed-off-by: Shengmin Gong <shengmin.gong@gmail.com>
Signed-off-by: John Gong <johngong0791@gmail.com>
---
virt/kvm/arm/vgic/vgic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
index abd9c7352677..d3cb1ce880e2 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -424,7 +424,7 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int intid,
return ret;
vcpu = kvm_get_vcpu(kvm, cpuid);
- if (!vcpu && intid < VGIC_NR_PRIVATE_IRQS)
+ if (!vcpu)
return -EINVAL;
irq = vgic_get_irq(kvm, vcpu, intid);
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: arm64: fix potential bug
2019-03-12 0:32 [PATCH] KVM: arm64: fix potential bug John Gong
@ 2019-03-12 1:42 ` André Przywara
2019-03-12 3:15 ` Gong John
0 siblings, 1 reply; 4+ messages in thread
From: André Przywara @ 2019-03-12 1:42 UTC (permalink / raw)
To: John Gong, christoffer.dall
Cc: Shengmin Gong, Marc Zyngier, Julien Thierry, Jia He,
linux-arm-kernel, kvmarm, linux-kernel
On 12/03/2019 00:32, John Gong wrote:
Hi,
> Since intid always >= VGIC_NR_PRIVATE_IRQS,
How so? The PMU and the arch timer emulation use PPIs, so intid is
definitely < VGIC_NR_PRIVATE_IRQS there.
> so then even vcpu == NULL, it never return -EINVAL.
I am not sure I follow.
To uniquely identify an SPI interrupt, we just need the interrupt ID
(which is always >= 32). For PPIs and SGIs, we additionally need the
vCPU ID this private interrupt belongs to, as there are multiple
interrupts with the same INTID (one per VCPU).
The VCPU ID passed in for SPIs is just a dummy value (because we use the
same function to inject private and shared interrupts), so we don't need
to check for its validity.
Cheers,
Andre.
>
> Signed-off-by: Shengmin Gong <shengmin.gong@gmail.com>
> Signed-off-by: John Gong <johngong0791@gmail.com>
> ---
> virt/kvm/arm/vgic/vgic.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
> index abd9c7352677..d3cb1ce880e2 100644
> --- a/virt/kvm/arm/vgic/vgic.c
> +++ b/virt/kvm/arm/vgic/vgic.c
> @@ -424,7 +424,7 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int intid,
> return ret;
>
> vcpu = kvm_get_vcpu(kvm, cpuid);
> - if (!vcpu && intid < VGIC_NR_PRIVATE_IRQS)
> + if (!vcpu)
> return -EINVAL;
>
> irq = vgic_get_irq(kvm, vcpu, intid);
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: arm64: fix potential bug
2019-03-12 1:42 ` André Przywara
@ 2019-03-12 3:15 ` Gong John
2019-03-12 9:52 ` Andre Przywara
0 siblings, 1 reply; 4+ messages in thread
From: Gong John @ 2019-03-12 3:15 UTC (permalink / raw)
To: André Przywara
Cc: christoffer.dall, Shengmin Gong, Marc Zyngier, Julien Thierry,
Jia He, linux-arm-kernel, kvmarm, linux-kernel
Hi Andre,
On Tue, Mar 12, 2019 at 9:44 AM André Przywara <andre.przywara@arm.com> wrote:
>
> On 12/03/2019 00:32, John Gong wrote:
> Hi,
>
> > Since intid always >= VGIC_NR_PRIVATE_IRQS,
>
> How so? The PMU and the arch timer emulation use PPIs, so intid is
> definitely < VGIC_NR_PRIVATE_IRQS there.
>
> > so then even vcpu == NULL, it never return -EINVAL.
>
> I am not sure I follow.
> To uniquely identify an SPI interrupt, we just need the interrupt ID
> (which is always >= 32). For PPIs and SGIs, we additionally need the
> vCPU ID this private interrupt belongs to, as there are multiple
> interrupts with the same INTID (one per VCPU).
> The VCPU ID passed in for SPIs is just a dummy value (because we use the
> same function to inject private and shared interrupts), so we don't need
> to check for its validity.
>
> Cheers,
> Andre.
>
Thanks for your explanation. It's my fault to not consider the PPIs
and SGIs injection.
Sorry for polluting the mail list.
Cheers,
John Gong
> >
> > Signed-off-by: Shengmin Gong <shengmin.gong@gmail.com>
> > Signed-off-by: John Gong <johngong0791@gmail.com>
> > ---
> > virt/kvm/arm/vgic/vgic.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
> > index abd9c7352677..d3cb1ce880e2 100644
> > --- a/virt/kvm/arm/vgic/vgic.c
> > +++ b/virt/kvm/arm/vgic/vgic.c
> > @@ -424,7 +424,7 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int intid,
> > return ret;
> >
> > vcpu = kvm_get_vcpu(kvm, cpuid);
> > - if (!vcpu && intid < VGIC_NR_PRIVATE_IRQS)
> > + if (!vcpu)
> > return -EINVAL;
> >
> > irq = vgic_get_irq(kvm, vcpu, intid);
> >
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: arm64: fix potential bug
2019-03-12 3:15 ` Gong John
@ 2019-03-12 9:52 ` Andre Przywara
0 siblings, 0 replies; 4+ messages in thread
From: Andre Przywara @ 2019-03-12 9:52 UTC (permalink / raw)
To: Gong John
Cc: christoffer.dall, Shengmin Gong, Marc Zyngier, Julien Thierry,
Jia He, linux-arm-kernel, kvmarm, linux-kernel
On Tue, 12 Mar 2019 11:15:14 +0800
Gong John <johngong0791@gmail.com> wrote:
Hi,
> On Tue, Mar 12, 2019 at 9:44 AM André Przywara <andre.przywara@arm.com> wrote:
> >
> > On 12/03/2019 00:32, John Gong wrote:
> > Hi,
> >
> > > Since intid always >= VGIC_NR_PRIVATE_IRQS,
> >
> > How so? The PMU and the arch timer emulation use PPIs, so intid is
> > definitely < VGIC_NR_PRIVATE_IRQS there.
> >
> > > so then even vcpu == NULL, it never return -EINVAL.
> >
> > I am not sure I follow.
> > To uniquely identify an SPI interrupt, we just need the interrupt ID
> > (which is always >= 32). For PPIs and SGIs, we additionally need the
> > vCPU ID this private interrupt belongs to, as there are multiple
> > interrupts with the same INTID (one per VCPU).
> > The VCPU ID passed in for SPIs is just a dummy value (because we use the
> > same function to inject private and shared interrupts), so we don't need
> > to check for its validity.
> >
> > Cheers,
> > Andre.
> >
> Thanks for your explanation. It's my fault to not consider the PPIs
> and SGIs injection.
Don't worry, we are glad when people actually look at the code. And we rather have a false positive report than a bug slipping through.
Cheers,
Andre.
> Sorry for polluting the mail list.
>
> Cheers,
> John Gong
> > >
> > > Signed-off-by: Shengmin Gong <shengmin.gong@gmail.com>
> > > Signed-off-by: John Gong <johngong0791@gmail.com>
> > > ---
> > > virt/kvm/arm/vgic/vgic.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
> > > index abd9c7352677..d3cb1ce880e2 100644
> > > --- a/virt/kvm/arm/vgic/vgic.c
> > > +++ b/virt/kvm/arm/vgic/vgic.c
> > > @@ -424,7 +424,7 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int intid,
> > > return ret;
> > >
> > > vcpu = kvm_get_vcpu(kvm, cpuid);
> > > - if (!vcpu && intid < VGIC_NR_PRIVATE_IRQS)
> > > + if (!vcpu)
> > > return -EINVAL;
> > >
> > > irq = vgic_get_irq(kvm, vcpu, intid);
> > >
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-03-12 9:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-12 0:32 [PATCH] KVM: arm64: fix potential bug John Gong
2019-03-12 1:42 ` André Przywara
2019-03-12 3:15 ` Gong John
2019-03-12 9:52 ` Andre Przywara
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).