Restoring a corrupted collection entry is being ignored and treated as
success. More specifically, 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 | 35 ++++++++++++++++++++++++----------
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index fb2d26a73880..86c26aaa8275 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -999,15 +999,16 @@ static bool vgic_its_check_event_id(struct vgic_its *its, struct its_device *dev
return __is_visible_gfn_locked(its, gpa);
}
+/*
+ * Adds a new collection into the ITS collection table.
+ * Returns 0 on success, and a negative error value for generic errors.
+ */
static int vgic_its_alloc_collection(struct vgic_its *its,
struct its_collection **colp,
u32 coll_id)
{
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;
@@ -1101,7 +1102,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;
@@ -1256,6 +1262,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)
@@ -2497,6 +2507,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;
@@ -2511,7 +2525,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;
@@ -2523,11 +2537,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;
}
/**
@@ -2593,15 +2611,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.464.gb9c8b46e94-goog
