Restoring a corrupted collection entry is being ignored: a vgic_its_restore_cte failure is treated as success by vgic_its_restore_collection_table. vgic_its_restore_cte uses a positive number to return ITS error codes, and +1 to return success. The caller then uses "ret > 0" to check for success. An additional issue is that invalid entries return 0 and although that doesn't fail the restore, it leads to skipping all the next entries. Fix this by having vgic_its_restore_cte return negative numbers on error, and 0 on success (which includes skipping an invalid entry). While doing that, also fix alloc_collection return codes to not mix ITS error codes (positive numbers) and generic error codes (negative numbers). Signed-off-by: Ricardo Koller <ricarkol@xxxxxxxxxx> --- arch/arm64/kvm/vgic/vgic-its.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c index dfd73fa1ed43..4ece649e2493 100644 --- a/arch/arm64/kvm/vgic/vgic-its.c +++ b/arch/arm64/kvm/vgic/vgic-its.c @@ -1013,9 +1013,6 @@ static int vgic_its_alloc_collection(struct vgic_its *its, { struct its_collection *collection; - if (!vgic_its_check_id(its, its->baser_coll_table, coll_id, NULL)) - return E_ITS_MAPC_COLLECTION_OOR; - collection = kzalloc(sizeof(*collection), GFP_KERNEL_ACCOUNT); if (!collection) return -ENOMEM; @@ -1112,7 +1109,12 @@ static int vgic_its_cmd_handle_mapi(struct kvm *kvm, struct vgic_its *its, collection = find_collection(its, coll_id); if (!collection) { - int ret = vgic_its_alloc_collection(its, &collection, coll_id); + int ret; + + if (!vgic_its_check_id(its, its->baser_coll_table, coll_id, NULL)) + return E_ITS_MAPC_COLLECTION_OOR; + + ret = vgic_its_alloc_collection(its, &collection, coll_id); if (ret) return ret; new_coll = collection; @@ -1267,6 +1269,10 @@ static int vgic_its_cmd_handle_mapc(struct kvm *kvm, struct vgic_its *its, if (!collection) { int ret; + if (!vgic_its_check_id(its, its->baser_coll_table, + coll_id, NULL)) + return E_ITS_MAPC_COLLECTION_OOR; + ret = vgic_its_alloc_collection(its, &collection, coll_id); if (ret) @@ -2508,6 +2514,10 @@ static int vgic_its_save_cte(struct vgic_its *its, return kvm_write_guest_lock(its->dev->kvm, gpa, &val, esz); } +/* + * Restores a collection entry into the ITS collection table. + * Returns 0 on success, and a negative error value for generic errors. + */ static int vgic_its_restore_cte(struct vgic_its *its, gpa_t gpa, int esz) { struct its_collection *collection; @@ -2522,7 +2532,7 @@ static int vgic_its_restore_cte(struct vgic_its *its, gpa_t gpa, int esz) return ret; val = le64_to_cpu(val); if (!(val & KVM_ITS_CTE_VALID_MASK)) - return 0; + return 0; /* invalid entry, skip it */ target_addr = (u32)(val >> KVM_ITS_CTE_RDBASE_SHIFT); coll_id = val & KVM_ITS_CTE_ICID_MASK; @@ -2534,11 +2544,15 @@ static int vgic_its_restore_cte(struct vgic_its *its, gpa_t gpa, int esz) collection = find_collection(its, coll_id); if (collection) return -EEXIST; + + if (!vgic_its_check_id(its, its->baser_coll_table, coll_id, NULL)) + return -EINVAL; + ret = vgic_its_alloc_collection(its, &collection, coll_id); if (ret) return ret; collection->target_addr = target_addr; - return 1; + return 0; } /** @@ -2604,15 +2618,12 @@ static int vgic_its_restore_collection_table(struct vgic_its *its) while (read < max_size) { ret = vgic_its_restore_cte(its, gpa, cte_esz); - if (ret <= 0) + if (ret < 0) break; gpa += cte_esz; read += cte_esz; } - if (ret > 0) - return 0; - return ret; } -- 2.36.0.rc2.479.g8af0fa9b8e-goog _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm