Re: [Xen-devel] [PATCH v2 09/27] ARM: GICv3: introduce separate pending_irq structs for LPIs

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 24 Mar 2017, Andre Przywara wrote:
> >> +struct pending_irq *lpi_to_pending(struct vcpu *v, unsigned int lpi,
> >> +                                   bool allocate)
> >> +{
> >> +    struct lpi_pending_irq *lpi_irq, *empty = NULL;
> >> +
> >> +    spin_lock(&v->arch.vgic.pending_lpi_list_lock);
> >> +    list_for_each_entry(lpi_irq, &v->arch.vgic.pending_lpi_list, entry)
> >> +    {
> >> +        if ( lpi_irq->pirq.irq == lpi )
> >> +        {
> >> +            spin_unlock(&v->arch.vgic.pending_lpi_list_lock);
> >> +            return &lpi_irq->pirq;
> >> +        }
> >> +
> >> +        if ( lpi_irq->pirq.irq == 0 && !empty )
> >> +            empty = lpi_irq;
> >> +    }
> >> +
> >> +    if ( !allocate )
> >> +    {
> >> +        spin_unlock(&v->arch.vgic.pending_lpi_list_lock);
> >> +        return NULL;
> >> +    }
> >> +
> >> +    if ( !empty )
> >> +    {
> >> +        empty = xzalloc(struct lpi_pending_irq);
> > 
> > xzalloc will return NULL if memory is exhausted. There is a general lack
> > of error checking within this series. Any missing error could be a
> > potential target from a guest, leading to security issue. Stefano and I
> > already spot some, it does not mean we found all. So Before sending the
> > next version, please go through the series and verify *every* return.
> > 
> > Also, I can't find the code which release LPIs neither in this patch nor
> > in this series. A general rule is too have allocation and free within
> > the same patch. It is much easier to spot missing free.
> 
> There is no such code, on purpose. We only grow the number, but never
> shrink it (to what?, where to stop?, what if we need more again?). As
> said above, in the worst case this ends up at something where a static
> allocation would have started with from the beginning.

Dellocate struct pending_irq when the domain is destroyed or when an LPI
is disabled?

> to what?, where to stop?

We don't stop, why stop? Let's go back to 0 if we can.

> what if we need more again?

We allocate them again?

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel