KVM: arm64: vgic: Do not ignore vgic_its_restore_cte failures

Restoring a corrupted collection entry (like an out of range ID) is
being ignored and treated as success. More specifically, a
vgic_its_restore_cte failure is treated as success by
vgic_its_restore_collection_table.  vgic_its_restore_cte uses positive
and negative numbers to return error, and +1 to return success.  The
caller then uses "ret > 0" to check for success.

Fix this by having vgic_its_restore_cte only return negative numbers on
error.  Do this by changing alloc_collection return codes to only return
negative numbers on error.

Signed-off-by: Ricardo Koller <ricarkol@google.com>
Reviewed-by: Oliver Upton <oupton@google.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20220510001633.552496-4-ricarkol@google.com

diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index 8a7db83..f34e09c 100644 (file)
--- 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);
 }
 
+/*
+ * Add 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)
@@ -2491,6 +2501,11 @@ static int vgic_its_save_cte(struct vgic_its *its,
        return kvm_write_guest_lock(its->dev->kvm, gpa, &val, esz);
 }
 
+/*
+ * Restore a collection entry into the ITS collection table.
+ * Return +1 on success, 0 if the entry was invalid (which should be
+ * interpreted as end-of-table), 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;
@@ -2517,6 +2532,10 @@ 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;