Re: [PATCH 3/3] KVM: arm64: vgic-its: Don't write past the end of the collection table

Fuad Tabba <[email protected]> Wed, 5 Aug 2026 11:49:00 +0100
Newsgroups dev.linux.lists.kvmarm,dev.linux.lists.sashiko-reviews
Message-ID <CA+EHjTxuYLBwNsDnZd4B_jYWBDvGxXMWSDKPiMqjJjdCMYySaw@mail.gmail.com>
Sashiko my good LLM friend,

On Wed, 5 Aug 2026 at 10:55, <[email protected]> wrote:
...
> > diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
> > index ed281fbf008b9..c709fc5e17cda 100644
> > --- a/arch/arm64/kvm/vgic/vgic-its.c
> > +++ b/arch/arm64/kvm/vgic/vgic-its.c
> > @@ -2540,6 +2540,9 @@ static int vgic_its_save_collection_table(struct vgic_its *its)
> >       max_size = GITS_BASER_NR_PAGES(baser) * SZ_64K;
> >
> >       list_for_each_entry(collection, &its->collection_list, coll_list) {
> > +             if (filled == max_size)
> > +                     return -EINVAL;
> > +
> >               ret = vgic_its_save_cte(its, collection, gpa);
>
> [Severity: Medium]
> This is a pre-existing issue, but does this code save collections with IDs
> that exceed the current GITS_BASER size?
>
> If a guest maps a collection with a large ID, and then shrinks the table size
> in GITS_BASER while keeping VALID=1, the collection list isn't freed. Since
> the total number of mapped collections might still be small enough to pass the
> filled == max_size check, it appears the collection with the large ID will
> be successfully saved here.
>
> On the destination side, when vgic_its_restore_cte() reads this entry, it
> calls vgic_its_check_id() which checks the ID against the shrunken boundaries,
> returning -EINVAL and failing the migration.

You're right, vgic_its_check_id() validates the collection ID against
both the 16-bit maximum and the current table capacity, so it's the
better check here:

+        if (!vgic_its_check_id(its, baser, collection->collection_id, NULL))
+            return -EINVAL;

This also bounds the walk, and the filled == max_size check can go.
I'll switch to it in the respin.

Cheers,
/fuad

>
> Should this path also validate the collection ID against the current table
> size, similar to how vgic_its_save_device_tables() uses vgic_its_check_id()?
>
> >               if (ret)
> >                       return ret;
> >               gpa += cte_esz;
> >               filled += cte_esz;
> >       }
>
> --
> Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3