@@ -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;
}
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@google.com> --- arch/arm64/kvm/vgic/vgic-its.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-)