linux-kernel - Re: [PATCH] KVM: arm64: fix potential bug

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 12 Mar 2019 09:52:22 +0000
From:   Andre Przywara <andre.przywara@....com>
To:     Gong John <johngong0791@...il.com>
Cc:     christoffer.dall@....com, Shengmin Gong <shengmin.gong@...il.com>,
        Marc Zyngier <marc.zyngier@....com>,
        Julien Thierry <julien.thierry@....com>,
        Jia He <hejianet@...il.com>,
        linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.cs.columbia.edu,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] KVM: arm64: fix potential bug

On Tue, 12 Mar 2019 11:15:14 +0800
Gong John <johngong0791@...il.com> wrote:

Hi,

> On Tue, Mar 12, 2019 at 9:44 AM André Przywara <andre.przywara@....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@...il.com>
> > > Signed-off-by: John Gong <johngong0791@...il.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);
> > >  
> >