On Wed, Aug 18, 2021 at 02:34:03PM -0700, Oliver Upton wrote:
> Hi Ricardo,
>
> On Wed, Aug 18, 2021 at 2:32 PM Ricardo Koller <[email protected]> wrote:
> >
> > vgic_get_irq(intid) is used all over the vgic code in order to get a
> > reference to a struct irq. It warns whenever intid is not a valid number
> > (like when it's a reserved IRQ number). The issue is that this warning
> > can be triggered from userspace (e.g., KVM_IRQ_LINE for intid 1020).
> >
> > Drop the WARN call from vgic_get_irq.
> >
> > Signed-off-by: Ricardo Koller <[email protected]>
> > ---
> > arch/arm64/kvm/vgic/vgic.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
> > index 111bff47e471..81cec508d413 100644
> > --- a/arch/arm64/kvm/vgic/vgic.c
> > +++ b/arch/arm64/kvm/vgic/vgic.c
> > @@ -106,7 +106,6 @@ struct vgic_irq *vgic_get_irq(struct kvm *kvm, struct
> > kvm_vcpu *vcpu,
> > if (intid >= VGIC_MIN_LPI)
> > return vgic_get_lpi(kvm, intid);
> >
> > - WARN(1, "Looking up struct vgic_irq for reserved INTID");
>
> Could we maybe downgrade the message to WARN_ONCE() (to get a stack)
> or pr_warn_ratelimited()? I agree it is problematic that userspace can
> cause this WARN to fire, but it'd be helpful for debugging too.
>
Was thinking about that, until I found this in bug.h:
/*
* WARN(), WARN_ON(), WARN_ON_ONCE, and so on can be used to report
* significant kernel issues that need prompt attention if they should
ever
* appear at runtime.
*
* Do not use these macros when checking for invalid external inputs
* (e.g. invalid system call arguments, or invalid data coming from
* network/devices),
Just in case, KVM_IRQ_LINE returns -EINVAL for an invalid intid (like
1020). I think it's more appropriate for the vmm to log it. What do you
think?
Thanks,
Ricardo
> --
> Thanks,
> Oliver
>
> > return NULL;
> > }
> >
> > --
> > 2.33.0.rc2.250.ged5fa647cd-goog
> >
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
