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