* [PATCH 5.15.y 1/3] KVM: arm64: vgic-its: Add a data length check in vgic_its_save_*
@ 2024-12-04 20:23 Jing Zhang
2024-12-04 20:23 ` [PATCH 5.15.y 2/3] KVM: arm64: vgic-its: Clear DTE when MAPD unmaps a device Jing Zhang
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Jing Zhang @ 2024-12-04 20:23 UTC (permalink / raw)
To: stable; +Cc: Marc Zyngier, Oliver Upton, Kunkun Jiang, Jing Zhang
commit 7fe28d7e68f92cc3d0668b8f2fbdf5c303ac3022 upstream.
In all the vgic_its_save_*() functinos, they do not check whether
the data length is 8 bytes before calling vgic_write_guest_lock.
This patch adds the check. To prevent the kernel from being blown up
when the fault occurs, KVM_BUG_ON() is used. And the other BUG_ON()s
are replaced together.
Cc: stable@vger.kernel.org
Signed-off-by: Kunkun Jiang <jiangkunkun@huawei.com>
[Jing: Update with the new entry read/write helpers]
Signed-off-by: Jing Zhang <jingzhangos@google.com>
Link: https://lore.kernel.org/r/20241107214137.428439-4-jingzhangos@google.com
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/kvm/vgic/vgic-its.c | 20 ++++++++------------
arch/arm64/kvm/vgic/vgic.h | 24 ++++++++++++++++++++++++
2 files changed, 32 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index 02ab6ab6ba91..bff7c49dfda5 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -2135,7 +2135,6 @@ static int scan_its_table(struct vgic_its *its, gpa_t base, int size, u32 esz,
static int vgic_its_save_ite(struct vgic_its *its, struct its_device *dev,
struct its_ite *ite, gpa_t gpa, int ite_esz)
{
- struct kvm *kvm = its->dev->kvm;
u32 next_offset;
u64 val;
@@ -2144,7 +2143,8 @@ static int vgic_its_save_ite(struct vgic_its *its, struct its_device *dev,
((u64)ite->irq->intid << KVM_ITS_ITE_PINTID_SHIFT) |
ite->collection->collection_id;
val = cpu_to_le64(val);
- return kvm_write_guest_lock(kvm, gpa, &val, ite_esz);
+
+ return vgic_its_write_entry_lock(its, gpa, val, ite_esz);
}
/**
@@ -2280,7 +2280,6 @@ static int vgic_its_restore_itt(struct vgic_its *its, struct its_device *dev)
static int vgic_its_save_dte(struct vgic_its *its, struct its_device *dev,
gpa_t ptr, int dte_esz)
{
- struct kvm *kvm = its->dev->kvm;
u64 val, itt_addr_field;
u32 next_offset;
@@ -2291,7 +2290,8 @@ static int vgic_its_save_dte(struct vgic_its *its, struct its_device *dev,
(itt_addr_field << KVM_ITS_DTE_ITTADDR_SHIFT) |
(dev->num_eventid_bits - 1));
val = cpu_to_le64(val);
- return kvm_write_guest_lock(kvm, ptr, &val, dte_esz);
+
+ return vgic_its_write_entry_lock(its, ptr, val, dte_esz);
}
/**
@@ -2471,7 +2471,8 @@ static int vgic_its_save_cte(struct vgic_its *its,
((u64)collection->target_addr << KVM_ITS_CTE_RDBASE_SHIFT) |
collection->collection_id);
val = cpu_to_le64(val);
- return kvm_write_guest_lock(its->dev->kvm, gpa, &val, esz);
+
+ return vgic_its_write_entry_lock(its, gpa, val, esz);
}
static int vgic_its_restore_cte(struct vgic_its *its, gpa_t gpa, int esz)
@@ -2482,8 +2483,7 @@ static int vgic_its_restore_cte(struct vgic_its *its, gpa_t gpa, int esz)
u64 val;
int ret;
- BUG_ON(esz > sizeof(val));
- ret = kvm_read_guest_lock(kvm, gpa, &val, esz);
+ ret = vgic_its_read_entry_lock(its, gpa, &val, esz);
if (ret)
return ret;
val = le64_to_cpu(val);
@@ -2517,7 +2517,6 @@ static int vgic_its_save_collection_table(struct vgic_its *its)
u64 baser = its->baser_coll_table;
gpa_t gpa = GITS_BASER_ADDR_48_to_52(baser);
struct its_collection *collection;
- u64 val;
size_t max_size, filled = 0;
int ret, cte_esz = abi->cte_esz;
@@ -2541,10 +2540,7 @@ static int vgic_its_save_collection_table(struct vgic_its *its)
* table is not fully filled, add a last dummy element
* with valid bit unset
*/
- val = 0;
- BUG_ON(cte_esz > sizeof(val));
- ret = kvm_write_guest_lock(its->dev->kvm, gpa, &val, cte_esz);
- return ret;
+ return vgic_its_write_entry_lock(its, gpa, 0, cte_esz);
}
/**
diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
index 2be4b0759f5b..d2fc5ecb223e 100644
--- a/arch/arm64/kvm/vgic/vgic.h
+++ b/arch/arm64/kvm/vgic/vgic.h
@@ -6,6 +6,7 @@
#define __KVM_ARM_VGIC_NEW_H__
#include <linux/irqchip/arm-gic-common.h>
+#include <asm/kvm_mmu.h>
#define PRODUCT_ID_KVM 0x4b /* ASCII code K */
#define IMPLEMENTER_ARM 0x43b
@@ -126,6 +127,29 @@ static inline bool vgic_irq_is_multi_sgi(struct vgic_irq *irq)
return vgic_irq_get_lr_count(irq) > 1;
}
+static inline int vgic_its_read_entry_lock(struct vgic_its *its, gpa_t eaddr,
+ u64 *eval, unsigned long esize)
+{
+ struct kvm *kvm = its->dev->kvm;
+
+ if (KVM_BUG_ON(esize != sizeof(*eval), kvm))
+ return -EINVAL;
+
+ return kvm_read_guest_lock(kvm, eaddr, eval, esize);
+
+}
+
+static inline int vgic_its_write_entry_lock(struct vgic_its *its, gpa_t eaddr,
+ u64 eval, unsigned long esize)
+{
+ struct kvm *kvm = its->dev->kvm;
+
+ if (KVM_BUG_ON(esize != sizeof(eval), kvm))
+ return -EINVAL;
+
+ return kvm_write_guest_lock(kvm, eaddr, &eval, esize);
+}
+
/*
* This struct provides an intermediate representation of the fields contained
* in the GICH_VMCR and ICH_VMCR registers, such that code exporting the GIC
base-commit: 0a51d2d4527b43c5e467ffa6897deefeaf499358
--
2.47.0.338.g60cca15819-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 5.15.y 2/3] KVM: arm64: vgic-its: Clear DTE when MAPD unmaps a device
2024-12-04 20:23 [PATCH 5.15.y 1/3] KVM: arm64: vgic-its: Add a data length check in vgic_its_save_* Jing Zhang
@ 2024-12-04 20:23 ` Jing Zhang
2024-12-04 22:11 ` Sasha Levin
2024-12-04 20:23 ` [PATCH 5.15.y 3/3] KVM: arm64: vgic-its: Clear ITE when DISCARD frees an ITE Jing Zhang
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Jing Zhang @ 2024-12-04 20:23 UTC (permalink / raw)
To: stable; +Cc: Marc Zyngier, Oliver Upton, Kunkun Jiang, Shusen Li, Jing Zhang
From: Kunkun Jiang <jiangkunkun@huawei.com>
commit e9649129d33dca561305fc590a7c4ba8c3e5675a upstream.
vgic_its_save_device_tables will traverse its->device_list to
save DTE for each device. vgic_its_restore_device_tables will
traverse each entry of device table and check if it is valid.
Restore if valid.
But when MAPD unmaps a device, it does not invalidate the
corresponding DTE. In the scenario of continuous saves
and restores, there may be a situation where a device's DTE
is not saved but is restored. This is unreasonable and may
cause restore to fail. This patch clears the corresponding
DTE when MAPD unmaps a device.
Cc: stable@vger.kernel.org
Fixes: 57a9a117154c ("KVM: arm64: vgic-its: Device table save/restore")
Co-developed-by: Shusen Li <lishusen2@huawei.com>
Signed-off-by: Shusen Li <lishusen2@huawei.com>
Signed-off-by: Kunkun Jiang <jiangkunkun@huawei.com>
[Jing: Update with entry write helper]
Signed-off-by: Jing Zhang <jingzhangos@google.com>
Link: https://lore.kernel.org/r/20241107214137.428439-5-jingzhangos@google.com
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/kvm/vgic/vgic-its.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index bff7c49dfda5..e1b2bbfe1fef 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -1182,9 +1182,11 @@ static int vgic_its_cmd_handle_mapd(struct kvm *kvm, struct vgic_its *its,
bool valid = its_cmd_get_validbit(its_cmd);
u8 num_eventid_bits = its_cmd_get_size(its_cmd);
gpa_t itt_addr = its_cmd_get_ittaddr(its_cmd);
+ int dte_esz = vgic_its_get_abi(its)->dte_esz;
struct its_device *device;
+ gpa_t gpa;
- if (!vgic_its_check_id(its, its->baser_device_table, device_id, NULL))
+ if (!vgic_its_check_id(its, its->baser_device_table, device_id, &gpa))
return E_ITS_MAPD_DEVICE_OOR;
if (valid && num_eventid_bits > VITS_TYPER_IDBITS)
@@ -1205,7 +1207,7 @@ static int vgic_its_cmd_handle_mapd(struct kvm *kvm, struct vgic_its *its,
* is an error, so we are done in any case.
*/
if (!valid)
- return 0;
+ return vgic_its_write_entry_lock(its, gpa, 0, dte_esz);
device = vgic_its_alloc_device(its, device_id, itt_addr,
num_eventid_bits);
--
2.47.0.338.g60cca15819-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 5.15.y 2/3] KVM: arm64: vgic-its: Clear DTE when MAPD unmaps a device
2024-12-04 20:23 ` [PATCH 5.15.y 2/3] KVM: arm64: vgic-its: Clear DTE when MAPD unmaps a device Jing Zhang
@ 2024-12-04 22:11 ` Sasha Levin
0 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2024-12-04 22:11 UTC (permalink / raw)
To: stable; +Cc: Jing Zhang, Sasha Levin
[ Sasha's backport helper bot ]
Hi,
The upstream commit SHA1 provided is correct: e9649129d33dca561305fc590a7c4ba8c3e5675a
WARNING: Author mismatch between patch and upstream commit:
Backport author: Jing Zhang <jingzhangos@google.com>
Commit author: Kunkun Jiang <jiangkunkun@huawei.com>
Status in newer kernel trees:
6.12.y | Present (different SHA1: 3b47865d395e)
6.11.y | Present (different SHA1: 78e981b6b725)
6.6.y | Present (different SHA1: 026af3ce08de)
6.1.y | Not found
5.15.y | Not found
Note: The patch differs from the upstream commit:
---
1: e9649129d33dc ! 1: 8e383a676fc79 KVM: arm64: vgic-its: Clear DTE when MAPD unmaps a device
@@ Metadata
## Commit message ##
KVM: arm64: vgic-its: Clear DTE when MAPD unmaps a device
+ commit e9649129d33dca561305fc590a7c4ba8c3e5675a upstream.
+
vgic_its_save_device_tables will traverse its->device_list to
save DTE for each device. vgic_its_restore_device_tables will
traverse each entry of device table and check if it is valid.
---
Results of testing on various branches:
| Branch | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-5.15.y | Success | Success |
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 5.15.y 3/3] KVM: arm64: vgic-its: Clear ITE when DISCARD frees an ITE
2024-12-04 20:23 [PATCH 5.15.y 1/3] KVM: arm64: vgic-its: Add a data length check in vgic_its_save_* Jing Zhang
2024-12-04 20:23 ` [PATCH 5.15.y 2/3] KVM: arm64: vgic-its: Clear DTE when MAPD unmaps a device Jing Zhang
@ 2024-12-04 20:23 ` Jing Zhang
2024-12-04 22:11 ` Sasha Levin
2024-12-04 22:12 ` [PATCH 5.15.y 1/3] KVM: arm64: vgic-its: Add a data length check in vgic_its_save_* Sasha Levin
2024-12-06 0:48 ` Oliver Upton
3 siblings, 1 reply; 7+ messages in thread
From: Jing Zhang @ 2024-12-04 20:23 UTC (permalink / raw)
To: stable; +Cc: Marc Zyngier, Oliver Upton, Kunkun Jiang, Jing Zhang
From: Kunkun Jiang <jiangkunkun@huawei.com>
commit 7602ffd1d5e8927fadd5187cb4aed2fdc9c47143 upstream.
When DISCARD frees an ITE, it does not invalidate the
corresponding ITE. In the scenario of continuous saves and
restores, there may be a situation where an ITE is not saved
but is restored. This is unreasonable and may cause restore
to fail. This patch clears the corresponding ITE when DISCARD
frees an ITE.
Cc: stable@vger.kernel.org
Fixes: eff484e0298d ("KVM: arm64: vgic-its: ITT save and restore")
Signed-off-by: Kunkun Jiang <jiangkunkun@huawei.com>
[Jing: Update with entry write helper]
Signed-off-by: Jing Zhang <jingzhangos@google.com>
Link: https://lore.kernel.org/r/20241107214137.428439-6-jingzhangos@google.com
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/kvm/vgic/vgic-its.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index e1b2bbfe1fef..4890131c87ef 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -855,6 +855,9 @@ static int vgic_its_cmd_handle_discard(struct kvm *kvm, struct vgic_its *its,
ite = find_ite(its, device_id, event_id);
if (ite && its_is_collection_mapped(ite->collection)) {
+ struct its_device *device = find_its_device(its, device_id);
+ int ite_esz = vgic_its_get_abi(its)->ite_esz;
+ gpa_t gpa = device->itt_addr + ite->event_id * ite_esz;
/*
* Though the spec talks about removing the pending state, we
* don't bother here since we clear the ITTE anyway and the
@@ -863,7 +866,8 @@ static int vgic_its_cmd_handle_discard(struct kvm *kvm, struct vgic_its *its,
vgic_its_invalidate_cache(kvm);
its_free_ite(kvm, ite);
- return 0;
+
+ return vgic_its_write_entry_lock(its, gpa, 0, ite_esz);
}
return E_ITS_DISCARD_UNMAPPED_INTERRUPT;
--
2.47.0.338.g60cca15819-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 5.15.y 3/3] KVM: arm64: vgic-its: Clear ITE when DISCARD frees an ITE
2024-12-04 20:23 ` [PATCH 5.15.y 3/3] KVM: arm64: vgic-its: Clear ITE when DISCARD frees an ITE Jing Zhang
@ 2024-12-04 22:11 ` Sasha Levin
0 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2024-12-04 22:11 UTC (permalink / raw)
To: stable; +Cc: Jing Zhang, Sasha Levin
[ Sasha's backport helper bot ]
Hi,
The upstream commit SHA1 provided is correct: 7602ffd1d5e8927fadd5187cb4aed2fdc9c47143
WARNING: Author mismatch between patch and upstream commit:
Backport author: Jing Zhang <jingzhangos@google.com>
Commit author: Kunkun Jiang <jiangkunkun@huawei.com>
Status in newer kernel trees:
6.12.y | Present (different SHA1: 98b18dff427f)
6.11.y | Present (different SHA1: 85272e522655)
6.6.y | Present (different SHA1: 9359737ade6c)
6.1.y | Present (different SHA1: 1f6c9a5c3b12)
5.15.y | Present (different SHA1: 04e670725c10)
Note: The patch differs from the upstream commit:
---
1: 7602ffd1d5e89 ! 1: 5a18cee073406 KVM: arm64: vgic-its: Clear ITE when DISCARD frees an ITE
@@ Metadata
## Commit message ##
KVM: arm64: vgic-its: Clear ITE when DISCARD frees an ITE
+ commit 7602ffd1d5e8927fadd5187cb4aed2fdc9c47143 upstream.
+
When DISCARD frees an ITE, it does not invalidate the
corresponding ITE. In the scenario of continuous saves and
restores, there may be a situation where an ITE is not saved
@@ arch/arm64/kvm/vgic/vgic-its.c: static int vgic_its_cmd_handle_discard(struct kv
* Though the spec talks about removing the pending state, we
* don't bother here since we clear the ITTE anyway and the
@@ arch/arm64/kvm/vgic/vgic-its.c: static int vgic_its_cmd_handle_discard(struct kvm *kvm, struct vgic_its *its,
- vgic_its_invalidate_cache(its);
+ vgic_its_invalidate_cache(kvm);
its_free_ite(kvm, ite);
- return 0;
---
Results of testing on various branches:
| Branch | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-5.15.y | Success | Success |
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 5.15.y 1/3] KVM: arm64: vgic-its: Add a data length check in vgic_its_save_*
2024-12-04 20:23 [PATCH 5.15.y 1/3] KVM: arm64: vgic-its: Add a data length check in vgic_its_save_* Jing Zhang
2024-12-04 20:23 ` [PATCH 5.15.y 2/3] KVM: arm64: vgic-its: Clear DTE when MAPD unmaps a device Jing Zhang
2024-12-04 20:23 ` [PATCH 5.15.y 3/3] KVM: arm64: vgic-its: Clear ITE when DISCARD frees an ITE Jing Zhang
@ 2024-12-04 22:12 ` Sasha Levin
2024-12-06 0:48 ` Oliver Upton
3 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2024-12-04 22:12 UTC (permalink / raw)
To: stable; +Cc: Jing Zhang, Sasha Levin
[ Sasha's backport helper bot ]
Hi,
The upstream commit SHA1 provided is correct: 7fe28d7e68f92cc3d0668b8f2fbdf5c303ac3022
Status in newer kernel trees:
6.12.y | Not found
6.11.y | Not found
6.6.y | Not found
6.1.y | Not found
5.15.y | Not found
Note: The patch differs from the upstream commit:
---
1: 7fe28d7e68f92 ! 1: e7bc6980df78e KVM: arm64: vgic-its: Add a data length check in vgic_its_save_*
@@ Metadata
## Commit message ##
KVM: arm64: vgic-its: Add a data length check in vgic_its_save_*
+ commit 7fe28d7e68f92cc3d0668b8f2fbdf5c303ac3022 upstream.
+
In all the vgic_its_save_*() functinos, they do not check whether
the data length is 8 bytes before calling vgic_write_guest_lock.
This patch adds the check. To prevent the kernel from being blown up
@@ arch/arm64/kvm/vgic/vgic-its.c: static int vgic_its_save_ite(struct vgic_its *it
((u64)ite->irq->intid << KVM_ITS_ITE_PINTID_SHIFT) |
ite->collection->collection_id;
val = cpu_to_le64(val);
-- return vgic_write_guest_lock(kvm, gpa, &val, ite_esz);
+- return kvm_write_guest_lock(kvm, gpa, &val, ite_esz);
+
+ return vgic_its_write_entry_lock(its, gpa, val, ite_esz);
}
@@ arch/arm64/kvm/vgic/vgic-its.c: static int vgic_its_save_dte(struct vgic_its *it
(itt_addr_field << KVM_ITS_DTE_ITTADDR_SHIFT) |
(dev->num_eventid_bits - 1));
val = cpu_to_le64(val);
-- return vgic_write_guest_lock(kvm, ptr, &val, dte_esz);
+- return kvm_write_guest_lock(kvm, ptr, &val, dte_esz);
+
+ return vgic_its_write_entry_lock(its, ptr, val, dte_esz);
}
@@ arch/arm64/kvm/vgic/vgic-its.c: static int vgic_its_save_cte(struct vgic_its *it
((u64)collection->target_addr << KVM_ITS_CTE_RDBASE_SHIFT) |
collection->collection_id);
val = cpu_to_le64(val);
-- return vgic_write_guest_lock(its->dev->kvm, gpa, &val, esz);
+- return kvm_write_guest_lock(its->dev->kvm, gpa, &val, esz);
+
+ return vgic_its_write_entry_lock(its, gpa, val, esz);
}
- /*
+ static int vgic_its_restore_cte(struct vgic_its *its, gpa_t gpa, int esz)
@@ arch/arm64/kvm/vgic/vgic-its.c: static int vgic_its_restore_cte(struct vgic_its *its, gpa_t gpa, int esz)
u64 val;
int ret;
@@ arch/arm64/kvm/vgic/vgic-its.c: static int vgic_its_save_collection_table(struct
*/
- val = 0;
- BUG_ON(cte_esz > sizeof(val));
-- ret = vgic_write_guest_lock(its->dev->kvm, gpa, &val, cte_esz);
+- ret = kvm_write_guest_lock(its->dev->kvm, gpa, &val, cte_esz);
- return ret;
+ return vgic_its_write_entry_lock(its, gpa, 0, cte_esz);
}
- /*
+ /**
## arch/arm64/kvm/vgic/vgic.h ##
-@@ arch/arm64/kvm/vgic/vgic.h: static inline int vgic_write_guest_lock(struct kvm *kvm, gpa_t gpa,
- return ret;
+@@
+ #define __KVM_ARM_VGIC_NEW_H__
+
+ #include <linux/irqchip/arm-gic-common.h>
++#include <asm/kvm_mmu.h>
+
+ #define PRODUCT_ID_KVM 0x4b /* ASCII code K */
+ #define IMPLEMENTER_ARM 0x43b
+@@ arch/arm64/kvm/vgic/vgic.h: static inline bool vgic_irq_is_multi_sgi(struct vgic_irq *irq)
+ return vgic_irq_get_lr_count(irq) > 1;
}
+static inline int vgic_its_read_entry_lock(struct vgic_its *its, gpa_t eaddr,
@@ arch/arm64/kvm/vgic/vgic.h: static inline int vgic_write_guest_lock(struct kvm *
+ if (KVM_BUG_ON(esize != sizeof(eval), kvm))
+ return -EINVAL;
+
-+ return vgic_write_guest_lock(kvm, eaddr, &eval, esize);
++ return kvm_write_guest_lock(kvm, eaddr, &eval, esize);
+}
+
/*
---
Results of testing on various branches:
| Branch | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-5.15.y | Success | Success |
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 5.15.y 1/3] KVM: arm64: vgic-its: Add a data length check in vgic_its_save_*
2024-12-04 20:23 [PATCH 5.15.y 1/3] KVM: arm64: vgic-its: Add a data length check in vgic_its_save_* Jing Zhang
` (2 preceding siblings ...)
2024-12-04 22:12 ` [PATCH 5.15.y 1/3] KVM: arm64: vgic-its: Add a data length check in vgic_its_save_* Sasha Levin
@ 2024-12-06 0:48 ` Oliver Upton
3 siblings, 0 replies; 7+ messages in thread
From: Oliver Upton @ 2024-12-06 0:48 UTC (permalink / raw)
To: Jing Zhang; +Cc: stable, Marc Zyngier, Kunkun Jiang
On Wed, Dec 04, 2024 at 12:23:38PM -0800, Jing Zhang wrote:
> commit 7fe28d7e68f92cc3d0668b8f2fbdf5c303ac3022 upstream.
>
> In all the vgic_its_save_*() functinos, they do not check whether
> the data length is 8 bytes before calling vgic_write_guest_lock.
> This patch adds the check. To prevent the kernel from being blown up
> when the fault occurs, KVM_BUG_ON() is used. And the other BUG_ON()s
> are replaced together.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Kunkun Jiang <jiangkunkun@huawei.com>
> [Jing: Update with the new entry read/write helpers]
> Signed-off-by: Jing Zhang <jingzhangos@google.com>
> Link: https://lore.kernel.org/r/20241107214137.428439-4-jingzhangos@google.com
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
For the series:
Acked-by: Oliver Upton <oliver.upton@linux.dev>
--
Thanks,
Oliver
> ---
> arch/arm64/kvm/vgic/vgic-its.c | 20 ++++++++------------
> arch/arm64/kvm/vgic/vgic.h | 24 ++++++++++++++++++++++++
> 2 files changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
> index 02ab6ab6ba91..bff7c49dfda5 100644
> --- a/arch/arm64/kvm/vgic/vgic-its.c
> +++ b/arch/arm64/kvm/vgic/vgic-its.c
> @@ -2135,7 +2135,6 @@ static int scan_its_table(struct vgic_its *its, gpa_t base, int size, u32 esz,
> static int vgic_its_save_ite(struct vgic_its *its, struct its_device *dev,
> struct its_ite *ite, gpa_t gpa, int ite_esz)
> {
> - struct kvm *kvm = its->dev->kvm;
> u32 next_offset;
> u64 val;
>
> @@ -2144,7 +2143,8 @@ static int vgic_its_save_ite(struct vgic_its *its, struct its_device *dev,
> ((u64)ite->irq->intid << KVM_ITS_ITE_PINTID_SHIFT) |
> ite->collection->collection_id;
> val = cpu_to_le64(val);
> - return kvm_write_guest_lock(kvm, gpa, &val, ite_esz);
> +
> + return vgic_its_write_entry_lock(its, gpa, val, ite_esz);
> }
>
> /**
> @@ -2280,7 +2280,6 @@ static int vgic_its_restore_itt(struct vgic_its *its, struct its_device *dev)
> static int vgic_its_save_dte(struct vgic_its *its, struct its_device *dev,
> gpa_t ptr, int dte_esz)
> {
> - struct kvm *kvm = its->dev->kvm;
> u64 val, itt_addr_field;
> u32 next_offset;
>
> @@ -2291,7 +2290,8 @@ static int vgic_its_save_dte(struct vgic_its *its, struct its_device *dev,
> (itt_addr_field << KVM_ITS_DTE_ITTADDR_SHIFT) |
> (dev->num_eventid_bits - 1));
> val = cpu_to_le64(val);
> - return kvm_write_guest_lock(kvm, ptr, &val, dte_esz);
> +
> + return vgic_its_write_entry_lock(its, ptr, val, dte_esz);
> }
>
> /**
> @@ -2471,7 +2471,8 @@ static int vgic_its_save_cte(struct vgic_its *its,
> ((u64)collection->target_addr << KVM_ITS_CTE_RDBASE_SHIFT) |
> collection->collection_id);
> val = cpu_to_le64(val);
> - return kvm_write_guest_lock(its->dev->kvm, gpa, &val, esz);
> +
> + return vgic_its_write_entry_lock(its, gpa, val, esz);
> }
>
> static int vgic_its_restore_cte(struct vgic_its *its, gpa_t gpa, int esz)
> @@ -2482,8 +2483,7 @@ static int vgic_its_restore_cte(struct vgic_its *its, gpa_t gpa, int esz)
> u64 val;
> int ret;
>
> - BUG_ON(esz > sizeof(val));
> - ret = kvm_read_guest_lock(kvm, gpa, &val, esz);
> + ret = vgic_its_read_entry_lock(its, gpa, &val, esz);
> if (ret)
> return ret;
> val = le64_to_cpu(val);
> @@ -2517,7 +2517,6 @@ static int vgic_its_save_collection_table(struct vgic_its *its)
> u64 baser = its->baser_coll_table;
> gpa_t gpa = GITS_BASER_ADDR_48_to_52(baser);
> struct its_collection *collection;
> - u64 val;
> size_t max_size, filled = 0;
> int ret, cte_esz = abi->cte_esz;
>
> @@ -2541,10 +2540,7 @@ static int vgic_its_save_collection_table(struct vgic_its *its)
> * table is not fully filled, add a last dummy element
> * with valid bit unset
> */
> - val = 0;
> - BUG_ON(cte_esz > sizeof(val));
> - ret = kvm_write_guest_lock(its->dev->kvm, gpa, &val, cte_esz);
> - return ret;
> + return vgic_its_write_entry_lock(its, gpa, 0, cte_esz);
> }
>
> /**
> diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
> index 2be4b0759f5b..d2fc5ecb223e 100644
> --- a/arch/arm64/kvm/vgic/vgic.h
> +++ b/arch/arm64/kvm/vgic/vgic.h
> @@ -6,6 +6,7 @@
> #define __KVM_ARM_VGIC_NEW_H__
>
> #include <linux/irqchip/arm-gic-common.h>
> +#include <asm/kvm_mmu.h>
>
> #define PRODUCT_ID_KVM 0x4b /* ASCII code K */
> #define IMPLEMENTER_ARM 0x43b
> @@ -126,6 +127,29 @@ static inline bool vgic_irq_is_multi_sgi(struct vgic_irq *irq)
> return vgic_irq_get_lr_count(irq) > 1;
> }
>
> +static inline int vgic_its_read_entry_lock(struct vgic_its *its, gpa_t eaddr,
> + u64 *eval, unsigned long esize)
> +{
> + struct kvm *kvm = its->dev->kvm;
> +
> + if (KVM_BUG_ON(esize != sizeof(*eval), kvm))
> + return -EINVAL;
> +
> + return kvm_read_guest_lock(kvm, eaddr, eval, esize);
> +
> +}
> +
> +static inline int vgic_its_write_entry_lock(struct vgic_its *its, gpa_t eaddr,
> + u64 eval, unsigned long esize)
> +{
> + struct kvm *kvm = its->dev->kvm;
> +
> + if (KVM_BUG_ON(esize != sizeof(eval), kvm))
> + return -EINVAL;
> +
> + return kvm_write_guest_lock(kvm, eaddr, &eval, esize);
> +}
> +
> /*
> * This struct provides an intermediate representation of the fields contained
> * in the GICH_VMCR and ICH_VMCR registers, such that code exporting the GIC
>
> base-commit: 0a51d2d4527b43c5e467ffa6897deefeaf499358
> --
> 2.47.0.338.g60cca15819-goog
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-12-06 0:48 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-04 20:23 [PATCH 5.15.y 1/3] KVM: arm64: vgic-its: Add a data length check in vgic_its_save_* Jing Zhang
2024-12-04 20:23 ` [PATCH 5.15.y 2/3] KVM: arm64: vgic-its: Clear DTE when MAPD unmaps a device Jing Zhang
2024-12-04 22:11 ` Sasha Levin
2024-12-04 20:23 ` [PATCH 5.15.y 3/3] KVM: arm64: vgic-its: Clear ITE when DISCARD frees an ITE Jing Zhang
2024-12-04 22:11 ` Sasha Levin
2024-12-04 22:12 ` [PATCH 5.15.y 1/3] KVM: arm64: vgic-its: Add a data length check in vgic_its_save_* Sasha Levin
2024-12-06 0:48 ` Oliver Upton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox