linux-kernel - [PATCH v5 02/10] KVM: arm/arm64: vgic-its: Fix vgic_its_restore_collection

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 23 Oct 2017 16:08:21 +0200
From:   Eric Auger <eric.auger@...hat.com>
To:     eric.auger.pro@...il.com, eric.auger@...hat.com,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        kvmarm@...ts.cs.columbia.edu, marc.zyngier@....com,
        cdall@...aro.org, peter.maydell@...aro.org, andre.przywara@....com,
        wanghaibin.wang@...wei.com
Cc:     wu.wubin@...wei.com, drjones@...hat.com, wei@...hat.com
Subject: [PATCH v5 02/10] KVM: arm/arm64: vgic-its: Fix vgic_its_restore_collection_table returned value

vgic_its_restore_cte returns +1 if the collection table entry
is valid and properly decoded. As a consequence, if the
collection table is fully filled with valid data that are
decoded without error, vgic_its_restore_collection_table()
returns +1.  This is wrong.

Let's use the standard C convention for both vgic_its_restore_cte
and vgic_its_restore_collection_table. vgic_its_restore_cte now
returns whether we have reached the end of the table in the @last
output parameter. vgic_its_restore_collection_table aborts in case
of error or if the end is found. Otherwise, it now returns 0.

Fixes: ea1ad53e1e31a3 (KVM: arm64: vgic-its: Collection table save/restore)
Signed-off-by: Eric Auger <eric.auger@...hat.com>
Reviewed-by: Christoffer Dall <christoffer.dall@...aro.org>

---

need to be CC'ed stable

v4 -> v5:
- added Christoffer R-b

v2 -> v3: creation
---
 virt/kvm/arm/vgic/vgic-its.c | 40 +++++++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
index d27a301..b0ba80f 100644
--- a/virt/kvm/arm/vgic/vgic-its.c
+++ b/virt/kvm/arm/vgic/vgic-its.c
@@ -2169,7 +2169,19 @@ static int vgic_its_save_cte(struct vgic_its *its,
 	return kvm_write_guest(its->dev->kvm, gpa, &val, esz);
 }
 
-static int vgic_its_restore_cte(struct vgic_its *its, gpa_t gpa, int esz)
+/**
+ * vgic_its_restore_cte - Restore a collection table entry
+ *
+ * @its: its handle
+ * @gpa: guest physical address of the entry
+ * @esz: entry size
+ * @last: output boolean indicating whether we have reached the
+ *       end of the collection table (ie. an invalid entry was decoded)
+ *
+ * Return: 0 upon success, < 0 on error
+ */
+static int vgic_its_restore_cte(struct vgic_its *its, gpa_t gpa, int esz,
+				bool *last)
 {
 	struct its_collection *collection;
 	struct kvm *kvm = its->dev->kvm;
@@ -2182,7 +2194,8 @@ static int vgic_its_restore_cte(struct vgic_its *its, gpa_t gpa, int esz)
 	if (ret)
 		return ret;
 	val = le64_to_cpu(val);
-	if (!(val & KVM_ITS_CTE_VALID_MASK))
+	*last = !(val & KVM_ITS_CTE_VALID_MASK);
+	if (*last)
 		return 0;
 
 	target_addr = (u32)(val >> KVM_ITS_CTE_RDBASE_SHIFT);
@@ -2198,7 +2211,7 @@ static int vgic_its_restore_cte(struct vgic_its *its, gpa_t gpa, int esz)
 	if (ret)
 		return ret;
 	collection->target_addr = target_addr;
-	return 1;
+	return 0;
 }
 
 /**
@@ -2243,8 +2256,13 @@ static int vgic_its_save_collection_table(struct vgic_its *its)
 
 /**
  * vgic_its_restore_collection_table - reads the collection table
- * in guest memory and restores the ITS internal state. Requires the
- * BASER registers to be restored before.
+ * in guest memory and restores the ITS collection cache.
+ *
+ * @its: its handle
+ *
+ * Requires the Collection BASER to be previously restored.
+ *
+ * Returns 0 on success or < 0 on error
  */
 static int vgic_its_restore_collection_table(struct vgic_its *its)
 {
@@ -2262,13 +2280,17 @@ static int vgic_its_restore_collection_table(struct vgic_its *its)
 	max_size = GITS_BASER_NR_PAGES(its->baser_coll_table) * SZ_64K;
 
 	while (read < max_size) {
-		ret = vgic_its_restore_cte(its, gpa, cte_esz);
-		if (ret <= 0)
-			break;
+		bool last;
+
+		ret = vgic_its_restore_cte(its, gpa, cte_esz, &last);
+		if (ret < 0 || last)
+			return ret;
 		gpa += cte_esz;
 		read += cte_esz;
 	}
-	return ret;
+
+	/* table was fully filled with valid entries, decoded without error */
+	return 0;
 }
 
 /**
-- 
2.5.5