[PATCH 0/2] Fix wild/dangling pointer in x86 ptp_kvm

[PATCH 0/2] Fix wild/dangling pointer in x86 ptp_kvm

[Zelin Deng]

When I was doing PTP_SYS_OFFSET_PRECISE ioctl in VM which has 128 vCPUs,
I got error returned occasionally. Then I checked the routine of
"getcrosststamp". I found in kvm_arch_ptp_get_crosststamp() of x86,
pvclock vcpu time info was got from hv_clock arrary which has only 64
elements. Hence this ioctl is executed on vCPU > 64, a wild/dangling
pointer will be got, which had casued the error.
To confirm this finding, I wrote a simple PTP_SYS_OFFSET_PRECISE ioctl
test and used "taskset -c n" to run the test, when it was executed on
vCPUs >= 64 it returned error.
This patchset exposes this_cpu_pvti() to get per cpu pvclock vcpu time
info of vCPUs >= 64 insdead of getting them from hv_clock arrary.

Zelin Deng (2):
  x86/kvmclock: Move this_cpu_pvti into kvmclock.h
  ptp: Fix ptp_kvm_getcrosststamp issue for x86 ptp_kvm

 arch/x86/include/asm/kvmclock.h | 14 ++++++++++++++
 arch/x86/kernel/kvmclock.c      | 13 ++-----------
 drivers/ptp/ptp_kvm_x86.c       |  9 ++-------
 3 files changed, 18 insertions(+), 18 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] x86/kvmclock: Move this_cpu_pvti into kvmclock.h

[Zelin Deng]

There're other modules might use hv_clock_per_cpu variable like ptp_kvm,
so move it into kvmclock.h and export the symbol to make it visiable to
other modules.

Signed-off-by: Zelin Deng <zelin.deng@linux.alibaba.com>
Cc: <stable@vger.kernel.org>
---
 arch/x86/include/asm/kvmclock.h | 14 ++++++++++++++
 arch/x86/kernel/kvmclock.c      | 13 ++-----------
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/kvmclock.h b/arch/x86/include/asm/kvmclock.h
index eceea92..fcd1ad6 100644
--- a/arch/x86/include/asm/kvmclock.h
+++ b/arch/x86/include/asm/kvmclock.h
@@ -2,6 +2,20 @@
 #ifndef _ASM_X86_KVM_CLOCK_H
 #define _ASM_X86_KVM_CLOCK_H
 
+#include <linux/percpu.h>
+
 extern struct clocksource kvm_clock;
 
+extern struct pvclock_vsyscall_time_info *hv_clock_per_cpu;
+
+static inline struct pvclock_vcpu_time_info *this_cpu_pvti(void)
+{
+	return &this_cpu_read(hv_clock_per_cpu)->pvti;
+}
+
+static inline struct pvclock_vsyscall_time_info *this_cpu_hvclock(void)
+{
+	return this_cpu_read(hv_clock_per_cpu);
+}
+
 #endif /* _ASM_X86_KVM_CLOCK_H */
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index ad273e5..6c6b1b3 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -49,18 +49,9 @@ static int __init parse_no_kvmclock_vsyscall(char *arg)
 static struct pvclock_vsyscall_time_info
 			hv_clock_boot[HVC_BOOT_ARRAY_SIZE] __bss_decrypted __aligned(PAGE_SIZE);
 static struct pvclock_wall_clock wall_clock __bss_decrypted;
-static DEFINE_PER_CPU(struct pvclock_vsyscall_time_info *, hv_clock_per_cpu);
 static struct pvclock_vsyscall_time_info *hvclock_mem;
-
-static inline struct pvclock_vcpu_time_info *this_cpu_pvti(void)
-{
-	return &this_cpu_read(hv_clock_per_cpu)->pvti;
-}
-
-static inline struct pvclock_vsyscall_time_info *this_cpu_hvclock(void)
-{
-	return this_cpu_read(hv_clock_per_cpu);
-}
+DEFINE_PER_CPU(struct pvclock_vsyscall_time_info *, hv_clock_per_cpu);
+EXPORT_SYMBOL_GPL(hv_clock_per_cpu);
 
 /*
  * The wallclock is the time of day when we booted. Since then, some time may
-- 
1.8.3.1

[PATCH 2/2] ptp: Fix ptp_kvm_getcrosststamp issue for x86 ptp_kvm

[Zelin Deng]

If PTP_SYS_OFFSET_PRECISE ioctl is executed on vCPU >= 64, struct
pvclock_vcpu_time_info *src which is got by "src = &hv_clock[cpu].pvti"
could be wild/dangling pointer, as hv_clock arrary has only
HVC_BOOT_ARRAY_SIZE (64) elements.
Therefore let's change it to "this_cpu_pvti()"

Fixes: 95a3d4454bb1 ("Switch kvmclock data to a PER_CPU variable")
Signed-off-by: Zelin Deng <zelin.deng@linux.alibaba.com>
Cc: <stable@vger.kernel.org>
---
 drivers/ptp/ptp_kvm_x86.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/ptp/ptp_kvm_x86.c b/drivers/ptp/ptp_kvm_x86.c
index 3dd519d..d0096cd 100644
--- a/drivers/ptp/ptp_kvm_x86.c
+++ b/drivers/ptp/ptp_kvm_x86.c
@@ -15,8 +15,6 @@
 #include <linux/ptp_clock_kernel.h>
 #include <linux/ptp_kvm.h>
 
-struct pvclock_vsyscall_time_info *hv_clock;
-
 static phys_addr_t clock_pair_gpa;
 static struct kvm_clock_pairing clock_pair;
 
@@ -28,8 +26,7 @@ int kvm_arch_ptp_init(void)
 		return -ENODEV;
 
 	clock_pair_gpa = slow_virt_to_phys(&clock_pair);
-	hv_clock = pvclock_get_pvti_cpu0_va();
-	if (!hv_clock)
+	if (!pvclock_get_pvti_cpu0_va())
 		return -ENODEV;
 
 	ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING, clock_pair_gpa,
@@ -64,10 +61,8 @@ int kvm_arch_ptp_get_crosststamp(u64 *cycle, struct timespec64 *tspec,
 	struct pvclock_vcpu_time_info *src;
 	unsigned int version;
 	long ret;
-	int cpu;
 
-	cpu = smp_processor_id();
-	src = &hv_clock[cpu].pvti;
+	src = this_cpu_pvti();
 
 	do {
 		/*
-- 
1.8.3.1

Re: [PATCH 0/2] Fix wild/dangling pointer in x86 ptp_kvm

[Paolo Bonzini]

On 29/09/21 07:13, Zelin Deng wrote:
> When I was doing PTP_SYS_OFFSET_PRECISE ioctl in VM which has 128 vCPUs,
> I got error returned occasionally. Then I checked the routine of
> "getcrosststamp". I found in kvm_arch_ptp_get_crosststamp() of x86,
> pvclock vcpu time info was got from hv_clock arrary which has only 64
> elements. Hence this ioctl is executed on vCPU > 64, a wild/dangling
> pointer will be got, which had casued the error.
> To confirm this finding, I wrote a simple PTP_SYS_OFFSET_PRECISE ioctl
> test and used "taskset -c n" to run the test, when it was executed on
> vCPUs >= 64 it returned error.
> This patchset exposes this_cpu_pvti() to get per cpu pvclock vcpu time
> info of vCPUs >= 64 insdead of getting them from hv_clock arrary.
> 
> Zelin Deng (2):
>    x86/kvmclock: Move this_cpu_pvti into kvmclock.h
>    ptp: Fix ptp_kvm_getcrosststamp issue for x86 ptp_kvm
> 
>   arch/x86/include/asm/kvmclock.h | 14 ++++++++++++++
>   arch/x86/kernel/kvmclock.c      | 13 ++-----------
>   drivers/ptp/ptp_kvm_x86.c       |  9 ++-------
>   3 files changed, 18 insertions(+), 18 deletions(-)
> 

Queued, thanks.

Paolo