* [PATCH] MIPS: KVM: Fix NULL pointer dereference
@ 2023-06-26 7:49 Huacai Chen
2023-06-26 10:28 ` Philippe Mathieu-Daudé
2023-06-26 10:33 ` Paolo Bonzini
0 siblings, 2 replies; 9+ messages in thread
From: Huacai Chen @ 2023-06-26 7:49 UTC (permalink / raw)
To: Thomas Bogendoerfer, Paolo Bonzini, Huacai Chen
Cc: linux-mips, kvm, Jiaxun Yang, Huacai Chen, stable
After commit 45c7e8af4a5e3f0bea4ac209 ("MIPS: Remove KVM_TE support") we
get a NULL pointer dereference when creating a KVM guest:
[ 146.243409] Starting KVM with MIPS VZ extensions
[ 149.849151] CPU 3 Unable to handle kernel paging request at virtual address 0000000000000300, epc == ffffffffc06356ec, ra == ffffffffc063568c
[ 149.849177] Oops[#1]:
[ 149.849182] CPU: 3 PID: 2265 Comm: qemu-system-mip Not tainted 6.4.0-rc3+ #1671
[ 149.849188] Hardware name: THTF CX TL630 Series/THTF-LS3A4000-7A1000-ML4A, BIOS KL4.1F.TF.D.166.201225.R 12/25/2020
[ 149.849192] $ 0 : 0000000000000000 000000007400cce0 0000000000400004 ffffffff8119c740
[ 149.849209] $ 4 : 000000007400cce1 000000007400cce1 0000000000000000 0000000000000000
[ 149.849221] $ 8 : 000000240058bb36 ffffffff81421ac0 0000000000000000 0000000000400dc0
[ 149.849233] $12 : 9800000102a07cc8 ffffffff80e40e38 0000000000000001 0000000000400dc0
[ 149.849245] $16 : 0000000000000000 9800000106cd0000 9800000106cd0000 9800000100cce000
[ 149.849257] $20 : ffffffffc0632b28 ffffffffc05b31b0 9800000100ccca00 0000000000400000
[ 149.849269] $24 : 9800000106cd09ce ffffffff802f69d0
[ 149.849281] $28 : 9800000102a04000 9800000102a07cd0 98000001106a8000 ffffffffc063568c
[ 149.849293] Hi : 00000335b2111e66
[ 149.849295] Lo : 6668d90061ae0ae9
[ 149.849298] epc : ffffffffc06356ec kvm_vz_vcpu_setup+0xc4/0x328 [kvm]
[ 149.849324] ra : ffffffffc063568c kvm_vz_vcpu_setup+0x64/0x328 [kvm]
[ 149.849336] Status: 7400cce3 KX SX UX KERNEL EXL IE
[ 149.849351] Cause : 1000000c (ExcCode 03)
[ 149.849354] BadVA : 0000000000000300
[ 149.849357] PrId : 0014c004 (ICT Loongson-3)
[ 149.849360] Modules linked in: kvm nfnetlink_queue nfnetlink_log nfnetlink fuse sha256_generic libsha256 cfg80211 rfkill binfmt_misc vfat fat snd_hda_codec_hdmi input_leds led_class snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hda_core snd_pcm snd_timer snd serio_raw xhci_pci radeon drm_suballoc_helper drm_display_helper xhci_hcd ip_tables x_tables
[ 149.849432] Process qemu-system-mip (pid: 2265, threadinfo=00000000ae2982d2, task=0000000038e09ad4, tls=000000ffeba16030)
[ 149.849439] Stack : 9800000000000003 9800000100ccca00 9800000100ccc000 ffffffffc062cef4
[ 149.849453] 9800000102a07d18 c89b63a7ab338e00 0000000000000000 ffffffff811a0000
[ 149.849465] 0000000000000000 9800000106cd0000 ffffffff80e59938 98000001106a8920
[ 149.849476] ffffffff80e57f30 ffffffffc062854c ffffffff811a0000 9800000102bf4240
[ 149.849488] ffffffffc05b0000 ffffffff80e3a798 000000ff78000000 000000ff78000010
[ 149.849500] 0000000000000255 98000001021f7de0 98000001023f0078 ffffffff81434000
[ 149.849511] 0000000000000000 0000000000000000 9800000102ae0000 980000025e92ae28
[ 149.849523] 0000000000000000 c89b63a7ab338e00 0000000000000001 ffffffff8119dce0
[ 149.849535] 000000ff78000010 ffffffff804f3d3c 9800000102a07eb0 0000000000000255
[ 149.849546] 0000000000000000 ffffffff8049460c 000000ff78000010 0000000000000255
[ 149.849558] ...
[ 149.849565] Call Trace:
[ 149.849567] [<ffffffffc06356ec>] kvm_vz_vcpu_setup+0xc4/0x328 [kvm]
[ 149.849586] [<ffffffffc062cef4>] kvm_arch_vcpu_create+0x184/0x228 [kvm]
[ 149.849605] [<ffffffffc062854c>] kvm_vm_ioctl+0x64c/0xf28 [kvm]
[ 149.849623] [<ffffffff805209c0>] sys_ioctl+0xc8/0x118
[ 149.849631] [<ffffffff80219eb0>] syscall_common+0x34/0x58
The root cause is the deletion of kvm_mips_commpage_init() leaves vcpu->
arch.cop0 NULL. So fix it by make cop0 from a pointer to an embed object.
Fixes: 45c7e8af4a5e3f0bea4ac209 ("MIPS: Remove KVM_TE support")
Cc: stable@vger.kernel.org
Suggested-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
arch/mips/include/asm/kvm_host.h | 6 +++---
arch/mips/kvm/emulate.c | 22 +++++++++++-----------
arch/mips/kvm/mips.c | 16 ++++++++--------
arch/mips/kvm/trace.h | 8 ++++----
arch/mips/kvm/vz.c | 20 ++++++++++----------
5 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 957121a495f0..04cedf9f8811 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -317,7 +317,7 @@ struct kvm_vcpu_arch {
unsigned int aux_inuse;
/* COP0 State */
- struct mips_coproc *cop0;
+ struct mips_coproc cop0;
/* Resume PC after MMIO completion */
unsigned long io_pc;
@@ -698,7 +698,7 @@ static inline bool kvm_mips_guest_can_have_fpu(struct kvm_vcpu_arch *vcpu)
static inline bool kvm_mips_guest_has_fpu(struct kvm_vcpu_arch *vcpu)
{
return kvm_mips_guest_can_have_fpu(vcpu) &&
- kvm_read_c0_guest_config1(vcpu->cop0) & MIPS_CONF1_FP;
+ kvm_read_c0_guest_config1(&vcpu->cop0) & MIPS_CONF1_FP;
}
static inline bool kvm_mips_guest_can_have_msa(struct kvm_vcpu_arch *vcpu)
@@ -710,7 +710,7 @@ static inline bool kvm_mips_guest_can_have_msa(struct kvm_vcpu_arch *vcpu)
static inline bool kvm_mips_guest_has_msa(struct kvm_vcpu_arch *vcpu)
{
return kvm_mips_guest_can_have_msa(vcpu) &&
- kvm_read_c0_guest_config3(vcpu->cop0) & MIPS_CONF3_MSA;
+ kvm_read_c0_guest_config3(&vcpu->cop0) & MIPS_CONF3_MSA;
}
struct kvm_mips_callbacks {
diff --git a/arch/mips/kvm/emulate.c b/arch/mips/kvm/emulate.c
index edaec93a1a1f..e64372b8f66a 100644
--- a/arch/mips/kvm/emulate.c
+++ b/arch/mips/kvm/emulate.c
@@ -312,7 +312,7 @@ int kvm_get_badinstrp(u32 *opc, struct kvm_vcpu *vcpu, u32 *out)
*/
int kvm_mips_count_disabled(struct kvm_vcpu *vcpu)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
return (vcpu->arch.count_ctl & KVM_REG_MIPS_COUNT_CTL_DC) ||
(kvm_read_c0_guest_cause(cop0) & CAUSEF_DC);
@@ -384,7 +384,7 @@ static inline ktime_t kvm_mips_count_time(struct kvm_vcpu *vcpu)
*/
static u32 kvm_mips_read_count_running(struct kvm_vcpu *vcpu, ktime_t now)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
ktime_t expires, threshold;
u32 count, compare;
int running;
@@ -444,7 +444,7 @@ static u32 kvm_mips_read_count_running(struct kvm_vcpu *vcpu, ktime_t now)
*/
u32 kvm_mips_read_count(struct kvm_vcpu *vcpu)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
/* If count disabled just read static copy of count */
if (kvm_mips_count_disabled(vcpu))
@@ -502,7 +502,7 @@ ktime_t kvm_mips_freeze_hrtimer(struct kvm_vcpu *vcpu, u32 *count)
static void kvm_mips_resume_hrtimer(struct kvm_vcpu *vcpu,
ktime_t now, u32 count)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
u32 compare;
u64 delta;
ktime_t expire;
@@ -603,7 +603,7 @@ int kvm_mips_restore_hrtimer(struct kvm_vcpu *vcpu, ktime_t before,
*/
void kvm_mips_write_count(struct kvm_vcpu *vcpu, u32 count)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
ktime_t now;
/* Calculate bias */
@@ -649,7 +649,7 @@ void kvm_mips_init_count(struct kvm_vcpu *vcpu, unsigned long count_hz)
*/
int kvm_mips_set_count_hz(struct kvm_vcpu *vcpu, s64 count_hz)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
int dc;
ktime_t now;
u32 count;
@@ -696,7 +696,7 @@ int kvm_mips_set_count_hz(struct kvm_vcpu *vcpu, s64 count_hz)
*/
void kvm_mips_write_compare(struct kvm_vcpu *vcpu, u32 compare, bool ack)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
int dc;
u32 old_compare = kvm_read_c0_guest_compare(cop0);
s32 delta = compare - old_compare;
@@ -779,7 +779,7 @@ void kvm_mips_write_compare(struct kvm_vcpu *vcpu, u32 compare, bool ack)
*/
static ktime_t kvm_mips_count_disable(struct kvm_vcpu *vcpu)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
u32 count;
ktime_t now;
@@ -806,7 +806,7 @@ static ktime_t kvm_mips_count_disable(struct kvm_vcpu *vcpu)
*/
void kvm_mips_count_disable_cause(struct kvm_vcpu *vcpu)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
kvm_set_c0_guest_cause(cop0, CAUSEF_DC);
if (!(vcpu->arch.count_ctl & KVM_REG_MIPS_COUNT_CTL_DC))
@@ -826,7 +826,7 @@ void kvm_mips_count_disable_cause(struct kvm_vcpu *vcpu)
*/
void kvm_mips_count_enable_cause(struct kvm_vcpu *vcpu)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
u32 count;
kvm_clear_c0_guest_cause(cop0, CAUSEF_DC);
@@ -852,7 +852,7 @@ void kvm_mips_count_enable_cause(struct kvm_vcpu *vcpu)
*/
int kvm_mips_set_count_ctl(struct kvm_vcpu *vcpu, s64 count_ctl)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
s64 changed = count_ctl ^ vcpu->arch.count_ctl;
s64 delta;
ktime_t expire, now;
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 884be4ef99dc..aa5583a7b05b 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -649,7 +649,7 @@ static int kvm_mips_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices)
static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
const struct kvm_one_reg *reg)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
struct mips_fpu_struct *fpu = &vcpu->arch.fpu;
int ret;
s64 v;
@@ -761,7 +761,7 @@ static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
static int kvm_mips_set_reg(struct kvm_vcpu *vcpu,
const struct kvm_one_reg *reg)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
struct mips_fpu_struct *fpu = &vcpu->arch.fpu;
s64 v;
s64 vs[2];
@@ -1086,7 +1086,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
{
return kvm_mips_pending_timer(vcpu) ||
- kvm_read_c0_guest_cause(vcpu->arch.cop0) & C_TI;
+ kvm_read_c0_guest_cause(&vcpu->arch.cop0) & C_TI;
}
int kvm_arch_vcpu_dump_regs(struct kvm_vcpu *vcpu)
@@ -1110,7 +1110,7 @@ int kvm_arch_vcpu_dump_regs(struct kvm_vcpu *vcpu)
kvm_debug("\thi: 0x%08lx\n", vcpu->arch.hi);
kvm_debug("\tlo: 0x%08lx\n", vcpu->arch.lo);
- cop0 = vcpu->arch.cop0;
+ cop0 = &vcpu->arch.cop0;
kvm_debug("\tStatus: 0x%08x, Cause: 0x%08x\n",
kvm_read_c0_guest_status(cop0),
kvm_read_c0_guest_cause(cop0));
@@ -1232,7 +1232,7 @@ static int __kvm_mips_handle_exit(struct kvm_vcpu *vcpu)
case EXCCODE_TLBS:
kvm_debug("TLB ST fault: cause %#x, status %#x, PC: %p, BadVaddr: %#lx\n",
- cause, kvm_read_c0_guest_status(vcpu->arch.cop0), opc,
+ cause, kvm_read_c0_guest_status(&vcpu->arch.cop0), opc,
badvaddr);
++vcpu->stat.tlbmiss_st_exits;
@@ -1304,7 +1304,7 @@ static int __kvm_mips_handle_exit(struct kvm_vcpu *vcpu)
kvm_get_badinstr(opc, vcpu, &inst);
kvm_err("Exception Code: %d, not yet handled, @ PC: %p, inst: 0x%08x BadVaddr: %#lx Status: %#x\n",
exccode, opc, inst, badvaddr,
- kvm_read_c0_guest_status(vcpu->arch.cop0));
+ kvm_read_c0_guest_status(&vcpu->arch.cop0));
kvm_arch_vcpu_dump_regs(vcpu);
run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
ret = RESUME_HOST;
@@ -1377,7 +1377,7 @@ int noinstr kvm_mips_handle_exit(struct kvm_vcpu *vcpu)
/* Enable FPU for guest and restore context */
void kvm_own_fpu(struct kvm_vcpu *vcpu)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
unsigned int sr, cfg5;
preempt_disable();
@@ -1421,7 +1421,7 @@ void kvm_own_fpu(struct kvm_vcpu *vcpu)
/* Enable MSA for guest and restore context */
void kvm_own_msa(struct kvm_vcpu *vcpu)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
unsigned int sr, cfg5;
preempt_disable();
diff --git a/arch/mips/kvm/trace.h b/arch/mips/kvm/trace.h
index a8c7fd7bf6d2..136c3535a1cb 100644
--- a/arch/mips/kvm/trace.h
+++ b/arch/mips/kvm/trace.h
@@ -322,11 +322,11 @@ TRACE_EVENT_FN(kvm_guest_mode_change,
),
TP_fast_assign(
- __entry->epc = kvm_read_c0_guest_epc(vcpu->arch.cop0);
+ __entry->epc = kvm_read_c0_guest_epc(&vcpu->arch.cop0);
__entry->pc = vcpu->arch.pc;
- __entry->badvaddr = kvm_read_c0_guest_badvaddr(vcpu->arch.cop0);
- __entry->status = kvm_read_c0_guest_status(vcpu->arch.cop0);
- __entry->cause = kvm_read_c0_guest_cause(vcpu->arch.cop0);
+ __entry->badvaddr = kvm_read_c0_guest_badvaddr(&vcpu->arch.cop0);
+ __entry->status = kvm_read_c0_guest_status(&vcpu->arch.cop0);
+ __entry->cause = kvm_read_c0_guest_cause(&vcpu->arch.cop0);
),
TP_printk("EPC: 0x%08lx PC: 0x%08lx Status: 0x%08x Cause: 0x%08x BadVAddr: 0x%08lx",
diff --git a/arch/mips/kvm/vz.c b/arch/mips/kvm/vz.c
index 3d21cbfa7443..99d5a71e4300 100644
--- a/arch/mips/kvm/vz.c
+++ b/arch/mips/kvm/vz.c
@@ -422,7 +422,7 @@ static void _kvm_vz_restore_htimer(struct kvm_vcpu *vcpu,
*/
static void kvm_vz_restore_timer(struct kvm_vcpu *vcpu)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
u32 cause, compare;
compare = kvm_read_sw_gc0_compare(cop0);
@@ -517,7 +517,7 @@ static void _kvm_vz_save_htimer(struct kvm_vcpu *vcpu,
*/
static void kvm_vz_save_timer(struct kvm_vcpu *vcpu)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
u32 gctl0, compare, cause;
gctl0 = read_c0_guestctl0();
@@ -863,7 +863,7 @@ static unsigned long mips_process_maar(unsigned int op, unsigned long val)
static void kvm_write_maari(struct kvm_vcpu *vcpu, unsigned long val)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
val &= MIPS_MAARI_INDEX;
if (val == MIPS_MAARI_INDEX)
@@ -876,7 +876,7 @@ static enum emulation_result kvm_vz_gpsi_cop0(union mips_instruction inst,
u32 *opc, u32 cause,
struct kvm_vcpu *vcpu)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
enum emulation_result er = EMULATE_DONE;
u32 rt, rd, sel;
unsigned long curr_pc;
@@ -1911,7 +1911,7 @@ static int kvm_vz_get_one_reg(struct kvm_vcpu *vcpu,
const struct kvm_one_reg *reg,
s64 *v)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
unsigned int idx;
switch (reg->id) {
@@ -2081,7 +2081,7 @@ static int kvm_vz_get_one_reg(struct kvm_vcpu *vcpu,
case KVM_REG_MIPS_CP0_MAARI:
if (!cpu_guest_has_maar || cpu_guest_has_dyn_maar)
return -EINVAL;
- *v = kvm_read_sw_gc0_maari(vcpu->arch.cop0);
+ *v = kvm_read_sw_gc0_maari(&vcpu->arch.cop0);
break;
#ifdef CONFIG_64BIT
case KVM_REG_MIPS_CP0_XCONTEXT:
@@ -2135,7 +2135,7 @@ static int kvm_vz_set_one_reg(struct kvm_vcpu *vcpu,
const struct kvm_one_reg *reg,
s64 v)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
unsigned int idx;
int ret = 0;
unsigned int cur, change;
@@ -2562,7 +2562,7 @@ static void kvm_vz_vcpu_load_tlb(struct kvm_vcpu *vcpu, int cpu)
static int kvm_vz_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
bool migrated, all;
/*
@@ -2704,7 +2704,7 @@ static int kvm_vz_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
static int kvm_vz_vcpu_put(struct kvm_vcpu *vcpu, int cpu)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
if (current->flags & PF_VCPU)
kvm_vz_vcpu_save_wired(vcpu);
@@ -3076,7 +3076,7 @@ static void kvm_vz_vcpu_uninit(struct kvm_vcpu *vcpu)
static int kvm_vz_vcpu_setup(struct kvm_vcpu *vcpu)
{
- struct mips_coproc *cop0 = vcpu->arch.cop0;
+ struct mips_coproc *cop0 = &vcpu->arch.cop0;
unsigned long count_hz = 100*1000*1000; /* default to 100 MHz */
/*
--
2.39.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] MIPS: KVM: Fix NULL pointer dereference
2023-06-26 7:49 [PATCH] MIPS: KVM: Fix NULL pointer dereference Huacai Chen
@ 2023-06-26 10:28 ` Philippe Mathieu-Daudé
2023-06-28 7:34 ` Huacai Chen
2023-06-26 10:33 ` Paolo Bonzini
1 sibling, 1 reply; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-06-26 10:28 UTC (permalink / raw)
To: Huacai Chen, Thomas Bogendoerfer, Paolo Bonzini, Huacai Chen
Cc: linux-mips, kvm, Jiaxun Yang, stable
On 26/6/23 09:49, Huacai Chen wrote:
> After commit 45c7e8af4a5e3f0bea4ac209 ("MIPS: Remove KVM_TE support") we
> get a NULL pointer dereference when creating a KVM guest:
>
> [ 146.243409] Starting KVM with MIPS VZ extensions
> [ 149.849151] CPU 3 Unable to handle kernel paging request at virtual address 0000000000000300, epc == ffffffffc06356ec, ra == ffffffffc063568c
> [ 149.849177] Oops[#1]:
> [ 149.849182] CPU: 3 PID: 2265 Comm: qemu-system-mip Not tainted 6.4.0-rc3+ #1671
> [ 149.849188] Hardware name: THTF CX TL630 Series/THTF-LS3A4000-7A1000-ML4A, BIOS KL4.1F.TF.D.166.201225.R 12/25/2020
> [ 149.849192] $ 0 : 0000000000000000 000000007400cce0 0000000000400004 ffffffff8119c740
> [ 149.849209] $ 4 : 000000007400cce1 000000007400cce1 0000000000000000 0000000000000000
> [ 149.849221] $ 8 : 000000240058bb36 ffffffff81421ac0 0000000000000000 0000000000400dc0
> [ 149.849233] $12 : 9800000102a07cc8 ffffffff80e40e38 0000000000000001 0000000000400dc0
> [ 149.849245] $16 : 0000000000000000 9800000106cd0000 9800000106cd0000 9800000100cce000
> [ 149.849257] $20 : ffffffffc0632b28 ffffffffc05b31b0 9800000100ccca00 0000000000400000
> [ 149.849269] $24 : 9800000106cd09ce ffffffff802f69d0
> [ 149.849281] $28 : 9800000102a04000 9800000102a07cd0 98000001106a8000 ffffffffc063568c
> [ 149.849293] Hi : 00000335b2111e66
> [ 149.849295] Lo : 6668d90061ae0ae9
> [ 149.849298] epc : ffffffffc06356ec kvm_vz_vcpu_setup+0xc4/0x328 [kvm]
> [ 149.849324] ra : ffffffffc063568c kvm_vz_vcpu_setup+0x64/0x328 [kvm]
> [ 149.849336] Status: 7400cce3 KX SX UX KERNEL EXL IE
> [ 149.849351] Cause : 1000000c (ExcCode 03)
> [ 149.849354] BadVA : 0000000000000300
> [ 149.849357] PrId : 0014c004 (ICT Loongson-3)
> [ 149.849360] Modules linked in: kvm nfnetlink_queue nfnetlink_log nfnetlink fuse sha256_generic libsha256 cfg80211 rfkill binfmt_misc vfat fat snd_hda_codec_hdmi input_leds led_class snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hda_core snd_pcm snd_timer snd serio_raw xhci_pci radeon drm_suballoc_helper drm_display_helper xhci_hcd ip_tables x_tables
> [ 149.849432] Process qemu-system-mip (pid: 2265, threadinfo=00000000ae2982d2, task=0000000038e09ad4, tls=000000ffeba16030)
> [ 149.849439] Stack : 9800000000000003 9800000100ccca00 9800000100ccc000 ffffffffc062cef4
> [ 149.849453] 9800000102a07d18 c89b63a7ab338e00 0000000000000000 ffffffff811a0000
> [ 149.849465] 0000000000000000 9800000106cd0000 ffffffff80e59938 98000001106a8920
> [ 149.849476] ffffffff80e57f30 ffffffffc062854c ffffffff811a0000 9800000102bf4240
> [ 149.849488] ffffffffc05b0000 ffffffff80e3a798 000000ff78000000 000000ff78000010
> [ 149.849500] 0000000000000255 98000001021f7de0 98000001023f0078 ffffffff81434000
> [ 149.849511] 0000000000000000 0000000000000000 9800000102ae0000 980000025e92ae28
> [ 149.849523] 0000000000000000 c89b63a7ab338e00 0000000000000001 ffffffff8119dce0
> [ 149.849535] 000000ff78000010 ffffffff804f3d3c 9800000102a07eb0 0000000000000255
> [ 149.849546] 0000000000000000 ffffffff8049460c 000000ff78000010 0000000000000255
> [ 149.849558] ...
> [ 149.849565] Call Trace:
> [ 149.849567] [<ffffffffc06356ec>] kvm_vz_vcpu_setup+0xc4/0x328 [kvm]
> [ 149.849586] [<ffffffffc062cef4>] kvm_arch_vcpu_create+0x184/0x228 [kvm]
> [ 149.849605] [<ffffffffc062854c>] kvm_vm_ioctl+0x64c/0xf28 [kvm]
> [ 149.849623] [<ffffffff805209c0>] sys_ioctl+0xc8/0x118
> [ 149.849631] [<ffffffff80219eb0>] syscall_common+0x34/0x58
>
> The root cause is the deletion of kvm_mips_commpage_init() leaves vcpu->
> arch.cop0 NULL. So fix it by make cop0 from a pointer to an embed object.
"by making ... to an embedded object."
>
> Fixes: 45c7e8af4a5e3f0bea4ac209 ("MIPS: Remove KVM_TE support")
> Cc: stable@vger.kernel.org
Reported-by: Yu Zhao <yuzhao@google.com>
> Suggested-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
> arch/mips/include/asm/kvm_host.h | 6 +++---
> arch/mips/kvm/emulate.c | 22 +++++++++++-----------
> arch/mips/kvm/mips.c | 16 ++++++++--------
> arch/mips/kvm/trace.h | 8 ++++----
> arch/mips/kvm/vz.c | 20 ++++++++++----------
> 5 files changed, 36 insertions(+), 36 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MIPS: KVM: Fix NULL pointer dereference
2023-06-26 7:49 [PATCH] MIPS: KVM: Fix NULL pointer dereference Huacai Chen
2023-06-26 10:28 ` Philippe Mathieu-Daudé
@ 2023-06-26 10:33 ` Paolo Bonzini
2023-06-26 12:44 ` Huacai Chen
` (2 more replies)
1 sibling, 3 replies; 9+ messages in thread
From: Paolo Bonzini @ 2023-06-26 10:33 UTC (permalink / raw)
To: Huacai Chen
Cc: Thomas Bogendoerfer, Huacai Chen, linux-mips, kvm, Jiaxun Yang, stable
On Mon, Jun 26, 2023 at 9:59 AM Huacai Chen <chenhuacai@loongson.cn> wrote:
>
> After commit 45c7e8af4a5e3f0bea4ac209 ("MIPS: Remove KVM_TE support") we
> get a NULL pointer dereference when creating a KVM guest:
To be honest, a bug that needed 2 years to be reproduced is probably a
sign that KVM/MIPS has no users. Any objections to removing it
altogether?
Paolo
>
> [ 146.243409] Starting KVM with MIPS VZ extensions
> [ 149.849151] CPU 3 Unable to handle kernel paging request at virtual address 0000000000000300, epc == ffffffffc06356ec, ra == ffffffffc063568c
> [ 149.849177] Oops[#1]:
> [ 149.849182] CPU: 3 PID: 2265 Comm: qemu-system-mip Not tainted 6.4.0-rc3+ #1671
> [ 149.849188] Hardware name: THTF CX TL630 Series/THTF-LS3A4000-7A1000-ML4A, BIOS KL4.1F.TF.D.166.201225.R 12/25/2020
> [ 149.849192] $ 0 : 0000000000000000 000000007400cce0 0000000000400004 ffffffff8119c740
> [ 149.849209] $ 4 : 000000007400cce1 000000007400cce1 0000000000000000 0000000000000000
> [ 149.849221] $ 8 : 000000240058bb36 ffffffff81421ac0 0000000000000000 0000000000400dc0
> [ 149.849233] $12 : 9800000102a07cc8 ffffffff80e40e38 0000000000000001 0000000000400dc0
> [ 149.849245] $16 : 0000000000000000 9800000106cd0000 9800000106cd0000 9800000100cce000
> [ 149.849257] $20 : ffffffffc0632b28 ffffffffc05b31b0 9800000100ccca00 0000000000400000
> [ 149.849269] $24 : 9800000106cd09ce ffffffff802f69d0
> [ 149.849281] $28 : 9800000102a04000 9800000102a07cd0 98000001106a8000 ffffffffc063568c
> [ 149.849293] Hi : 00000335b2111e66
> [ 149.849295] Lo : 6668d90061ae0ae9
> [ 149.849298] epc : ffffffffc06356ec kvm_vz_vcpu_setup+0xc4/0x328 [kvm]
> [ 149.849324] ra : ffffffffc063568c kvm_vz_vcpu_setup+0x64/0x328 [kvm]
> [ 149.849336] Status: 7400cce3 KX SX UX KERNEL EXL IE
> [ 149.849351] Cause : 1000000c (ExcCode 03)
> [ 149.849354] BadVA : 0000000000000300
> [ 149.849357] PrId : 0014c004 (ICT Loongson-3)
> [ 149.849360] Modules linked in: kvm nfnetlink_queue nfnetlink_log nfnetlink fuse sha256_generic libsha256 cfg80211 rfkill binfmt_misc vfat fat snd_hda_codec_hdmi input_leds led_class snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hda_core snd_pcm snd_timer snd serio_raw xhci_pci radeon drm_suballoc_helper drm_display_helper xhci_hcd ip_tables x_tables
> [ 149.849432] Process qemu-system-mip (pid: 2265, threadinfo=00000000ae2982d2, task=0000000038e09ad4, tls=000000ffeba16030)
> [ 149.849439] Stack : 9800000000000003 9800000100ccca00 9800000100ccc000 ffffffffc062cef4
> [ 149.849453] 9800000102a07d18 c89b63a7ab338e00 0000000000000000 ffffffff811a0000
> [ 149.849465] 0000000000000000 9800000106cd0000 ffffffff80e59938 98000001106a8920
> [ 149.849476] ffffffff80e57f30 ffffffffc062854c ffffffff811a0000 9800000102bf4240
> [ 149.849488] ffffffffc05b0000 ffffffff80e3a798 000000ff78000000 000000ff78000010
> [ 149.849500] 0000000000000255 98000001021f7de0 98000001023f0078 ffffffff81434000
> [ 149.849511] 0000000000000000 0000000000000000 9800000102ae0000 980000025e92ae28
> [ 149.849523] 0000000000000000 c89b63a7ab338e00 0000000000000001 ffffffff8119dce0
> [ 149.849535] 000000ff78000010 ffffffff804f3d3c 9800000102a07eb0 0000000000000255
> [ 149.849546] 0000000000000000 ffffffff8049460c 000000ff78000010 0000000000000255
> [ 149.849558] ...
> [ 149.849565] Call Trace:
> [ 149.849567] [<ffffffffc06356ec>] kvm_vz_vcpu_setup+0xc4/0x328 [kvm]
> [ 149.849586] [<ffffffffc062cef4>] kvm_arch_vcpu_create+0x184/0x228 [kvm]
> [ 149.849605] [<ffffffffc062854c>] kvm_vm_ioctl+0x64c/0xf28 [kvm]
> [ 149.849623] [<ffffffff805209c0>] sys_ioctl+0xc8/0x118
> [ 149.849631] [<ffffffff80219eb0>] syscall_common+0x34/0x58
>
> The root cause is the deletion of kvm_mips_commpage_init() leaves vcpu->
> arch.cop0 NULL. So fix it by make cop0 from a pointer to an embed object.
>
> Fixes: 45c7e8af4a5e3f0bea4ac209 ("MIPS: Remove KVM_TE support")
> Cc: stable@vger.kernel.org
> Suggested-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
> arch/mips/include/asm/kvm_host.h | 6 +++---
> arch/mips/kvm/emulate.c | 22 +++++++++++-----------
> arch/mips/kvm/mips.c | 16 ++++++++--------
> arch/mips/kvm/trace.h | 8 ++++----
> arch/mips/kvm/vz.c | 20 ++++++++++----------
> 5 files changed, 36 insertions(+), 36 deletions(-)
>
> diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
> index 957121a495f0..04cedf9f8811 100644
> --- a/arch/mips/include/asm/kvm_host.h
> +++ b/arch/mips/include/asm/kvm_host.h
> @@ -317,7 +317,7 @@ struct kvm_vcpu_arch {
> unsigned int aux_inuse;
>
> /* COP0 State */
> - struct mips_coproc *cop0;
> + struct mips_coproc cop0;
>
> /* Resume PC after MMIO completion */
> unsigned long io_pc;
> @@ -698,7 +698,7 @@ static inline bool kvm_mips_guest_can_have_fpu(struct kvm_vcpu_arch *vcpu)
> static inline bool kvm_mips_guest_has_fpu(struct kvm_vcpu_arch *vcpu)
> {
> return kvm_mips_guest_can_have_fpu(vcpu) &&
> - kvm_read_c0_guest_config1(vcpu->cop0) & MIPS_CONF1_FP;
> + kvm_read_c0_guest_config1(&vcpu->cop0) & MIPS_CONF1_FP;
> }
>
> static inline bool kvm_mips_guest_can_have_msa(struct kvm_vcpu_arch *vcpu)
> @@ -710,7 +710,7 @@ static inline bool kvm_mips_guest_can_have_msa(struct kvm_vcpu_arch *vcpu)
> static inline bool kvm_mips_guest_has_msa(struct kvm_vcpu_arch *vcpu)
> {
> return kvm_mips_guest_can_have_msa(vcpu) &&
> - kvm_read_c0_guest_config3(vcpu->cop0) & MIPS_CONF3_MSA;
> + kvm_read_c0_guest_config3(&vcpu->cop0) & MIPS_CONF3_MSA;
> }
>
> struct kvm_mips_callbacks {
> diff --git a/arch/mips/kvm/emulate.c b/arch/mips/kvm/emulate.c
> index edaec93a1a1f..e64372b8f66a 100644
> --- a/arch/mips/kvm/emulate.c
> +++ b/arch/mips/kvm/emulate.c
> @@ -312,7 +312,7 @@ int kvm_get_badinstrp(u32 *opc, struct kvm_vcpu *vcpu, u32 *out)
> */
> int kvm_mips_count_disabled(struct kvm_vcpu *vcpu)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>
> return (vcpu->arch.count_ctl & KVM_REG_MIPS_COUNT_CTL_DC) ||
> (kvm_read_c0_guest_cause(cop0) & CAUSEF_DC);
> @@ -384,7 +384,7 @@ static inline ktime_t kvm_mips_count_time(struct kvm_vcpu *vcpu)
> */
> static u32 kvm_mips_read_count_running(struct kvm_vcpu *vcpu, ktime_t now)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> ktime_t expires, threshold;
> u32 count, compare;
> int running;
> @@ -444,7 +444,7 @@ static u32 kvm_mips_read_count_running(struct kvm_vcpu *vcpu, ktime_t now)
> */
> u32 kvm_mips_read_count(struct kvm_vcpu *vcpu)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>
> /* If count disabled just read static copy of count */
> if (kvm_mips_count_disabled(vcpu))
> @@ -502,7 +502,7 @@ ktime_t kvm_mips_freeze_hrtimer(struct kvm_vcpu *vcpu, u32 *count)
> static void kvm_mips_resume_hrtimer(struct kvm_vcpu *vcpu,
> ktime_t now, u32 count)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> u32 compare;
> u64 delta;
> ktime_t expire;
> @@ -603,7 +603,7 @@ int kvm_mips_restore_hrtimer(struct kvm_vcpu *vcpu, ktime_t before,
> */
> void kvm_mips_write_count(struct kvm_vcpu *vcpu, u32 count)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> ktime_t now;
>
> /* Calculate bias */
> @@ -649,7 +649,7 @@ void kvm_mips_init_count(struct kvm_vcpu *vcpu, unsigned long count_hz)
> */
> int kvm_mips_set_count_hz(struct kvm_vcpu *vcpu, s64 count_hz)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> int dc;
> ktime_t now;
> u32 count;
> @@ -696,7 +696,7 @@ int kvm_mips_set_count_hz(struct kvm_vcpu *vcpu, s64 count_hz)
> */
> void kvm_mips_write_compare(struct kvm_vcpu *vcpu, u32 compare, bool ack)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> int dc;
> u32 old_compare = kvm_read_c0_guest_compare(cop0);
> s32 delta = compare - old_compare;
> @@ -779,7 +779,7 @@ void kvm_mips_write_compare(struct kvm_vcpu *vcpu, u32 compare, bool ack)
> */
> static ktime_t kvm_mips_count_disable(struct kvm_vcpu *vcpu)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> u32 count;
> ktime_t now;
>
> @@ -806,7 +806,7 @@ static ktime_t kvm_mips_count_disable(struct kvm_vcpu *vcpu)
> */
> void kvm_mips_count_disable_cause(struct kvm_vcpu *vcpu)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>
> kvm_set_c0_guest_cause(cop0, CAUSEF_DC);
> if (!(vcpu->arch.count_ctl & KVM_REG_MIPS_COUNT_CTL_DC))
> @@ -826,7 +826,7 @@ void kvm_mips_count_disable_cause(struct kvm_vcpu *vcpu)
> */
> void kvm_mips_count_enable_cause(struct kvm_vcpu *vcpu)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> u32 count;
>
> kvm_clear_c0_guest_cause(cop0, CAUSEF_DC);
> @@ -852,7 +852,7 @@ void kvm_mips_count_enable_cause(struct kvm_vcpu *vcpu)
> */
> int kvm_mips_set_count_ctl(struct kvm_vcpu *vcpu, s64 count_ctl)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> s64 changed = count_ctl ^ vcpu->arch.count_ctl;
> s64 delta;
> ktime_t expire, now;
> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> index 884be4ef99dc..aa5583a7b05b 100644
> --- a/arch/mips/kvm/mips.c
> +++ b/arch/mips/kvm/mips.c
> @@ -649,7 +649,7 @@ static int kvm_mips_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices)
> static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
> const struct kvm_one_reg *reg)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> struct mips_fpu_struct *fpu = &vcpu->arch.fpu;
> int ret;
> s64 v;
> @@ -761,7 +761,7 @@ static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
> static int kvm_mips_set_reg(struct kvm_vcpu *vcpu,
> const struct kvm_one_reg *reg)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> struct mips_fpu_struct *fpu = &vcpu->arch.fpu;
> s64 v;
> s64 vs[2];
> @@ -1086,7 +1086,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
> {
> return kvm_mips_pending_timer(vcpu) ||
> - kvm_read_c0_guest_cause(vcpu->arch.cop0) & C_TI;
> + kvm_read_c0_guest_cause(&vcpu->arch.cop0) & C_TI;
> }
>
> int kvm_arch_vcpu_dump_regs(struct kvm_vcpu *vcpu)
> @@ -1110,7 +1110,7 @@ int kvm_arch_vcpu_dump_regs(struct kvm_vcpu *vcpu)
> kvm_debug("\thi: 0x%08lx\n", vcpu->arch.hi);
> kvm_debug("\tlo: 0x%08lx\n", vcpu->arch.lo);
>
> - cop0 = vcpu->arch.cop0;
> + cop0 = &vcpu->arch.cop0;
> kvm_debug("\tStatus: 0x%08x, Cause: 0x%08x\n",
> kvm_read_c0_guest_status(cop0),
> kvm_read_c0_guest_cause(cop0));
> @@ -1232,7 +1232,7 @@ static int __kvm_mips_handle_exit(struct kvm_vcpu *vcpu)
>
> case EXCCODE_TLBS:
> kvm_debug("TLB ST fault: cause %#x, status %#x, PC: %p, BadVaddr: %#lx\n",
> - cause, kvm_read_c0_guest_status(vcpu->arch.cop0), opc,
> + cause, kvm_read_c0_guest_status(&vcpu->arch.cop0), opc,
> badvaddr);
>
> ++vcpu->stat.tlbmiss_st_exits;
> @@ -1304,7 +1304,7 @@ static int __kvm_mips_handle_exit(struct kvm_vcpu *vcpu)
> kvm_get_badinstr(opc, vcpu, &inst);
> kvm_err("Exception Code: %d, not yet handled, @ PC: %p, inst: 0x%08x BadVaddr: %#lx Status: %#x\n",
> exccode, opc, inst, badvaddr,
> - kvm_read_c0_guest_status(vcpu->arch.cop0));
> + kvm_read_c0_guest_status(&vcpu->arch.cop0));
> kvm_arch_vcpu_dump_regs(vcpu);
> run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
> ret = RESUME_HOST;
> @@ -1377,7 +1377,7 @@ int noinstr kvm_mips_handle_exit(struct kvm_vcpu *vcpu)
> /* Enable FPU for guest and restore context */
> void kvm_own_fpu(struct kvm_vcpu *vcpu)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> unsigned int sr, cfg5;
>
> preempt_disable();
> @@ -1421,7 +1421,7 @@ void kvm_own_fpu(struct kvm_vcpu *vcpu)
> /* Enable MSA for guest and restore context */
> void kvm_own_msa(struct kvm_vcpu *vcpu)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> unsigned int sr, cfg5;
>
> preempt_disable();
> diff --git a/arch/mips/kvm/trace.h b/arch/mips/kvm/trace.h
> index a8c7fd7bf6d2..136c3535a1cb 100644
> --- a/arch/mips/kvm/trace.h
> +++ b/arch/mips/kvm/trace.h
> @@ -322,11 +322,11 @@ TRACE_EVENT_FN(kvm_guest_mode_change,
> ),
>
> TP_fast_assign(
> - __entry->epc = kvm_read_c0_guest_epc(vcpu->arch.cop0);
> + __entry->epc = kvm_read_c0_guest_epc(&vcpu->arch.cop0);
> __entry->pc = vcpu->arch.pc;
> - __entry->badvaddr = kvm_read_c0_guest_badvaddr(vcpu->arch.cop0);
> - __entry->status = kvm_read_c0_guest_status(vcpu->arch.cop0);
> - __entry->cause = kvm_read_c0_guest_cause(vcpu->arch.cop0);
> + __entry->badvaddr = kvm_read_c0_guest_badvaddr(&vcpu->arch.cop0);
> + __entry->status = kvm_read_c0_guest_status(&vcpu->arch.cop0);
> + __entry->cause = kvm_read_c0_guest_cause(&vcpu->arch.cop0);
> ),
>
> TP_printk("EPC: 0x%08lx PC: 0x%08lx Status: 0x%08x Cause: 0x%08x BadVAddr: 0x%08lx",
> diff --git a/arch/mips/kvm/vz.c b/arch/mips/kvm/vz.c
> index 3d21cbfa7443..99d5a71e4300 100644
> --- a/arch/mips/kvm/vz.c
> +++ b/arch/mips/kvm/vz.c
> @@ -422,7 +422,7 @@ static void _kvm_vz_restore_htimer(struct kvm_vcpu *vcpu,
> */
> static void kvm_vz_restore_timer(struct kvm_vcpu *vcpu)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> u32 cause, compare;
>
> compare = kvm_read_sw_gc0_compare(cop0);
> @@ -517,7 +517,7 @@ static void _kvm_vz_save_htimer(struct kvm_vcpu *vcpu,
> */
> static void kvm_vz_save_timer(struct kvm_vcpu *vcpu)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> u32 gctl0, compare, cause;
>
> gctl0 = read_c0_guestctl0();
> @@ -863,7 +863,7 @@ static unsigned long mips_process_maar(unsigned int op, unsigned long val)
>
> static void kvm_write_maari(struct kvm_vcpu *vcpu, unsigned long val)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>
> val &= MIPS_MAARI_INDEX;
> if (val == MIPS_MAARI_INDEX)
> @@ -876,7 +876,7 @@ static enum emulation_result kvm_vz_gpsi_cop0(union mips_instruction inst,
> u32 *opc, u32 cause,
> struct kvm_vcpu *vcpu)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> enum emulation_result er = EMULATE_DONE;
> u32 rt, rd, sel;
> unsigned long curr_pc;
> @@ -1911,7 +1911,7 @@ static int kvm_vz_get_one_reg(struct kvm_vcpu *vcpu,
> const struct kvm_one_reg *reg,
> s64 *v)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> unsigned int idx;
>
> switch (reg->id) {
> @@ -2081,7 +2081,7 @@ static int kvm_vz_get_one_reg(struct kvm_vcpu *vcpu,
> case KVM_REG_MIPS_CP0_MAARI:
> if (!cpu_guest_has_maar || cpu_guest_has_dyn_maar)
> return -EINVAL;
> - *v = kvm_read_sw_gc0_maari(vcpu->arch.cop0);
> + *v = kvm_read_sw_gc0_maari(&vcpu->arch.cop0);
> break;
> #ifdef CONFIG_64BIT
> case KVM_REG_MIPS_CP0_XCONTEXT:
> @@ -2135,7 +2135,7 @@ static int kvm_vz_set_one_reg(struct kvm_vcpu *vcpu,
> const struct kvm_one_reg *reg,
> s64 v)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> unsigned int idx;
> int ret = 0;
> unsigned int cur, change;
> @@ -2562,7 +2562,7 @@ static void kvm_vz_vcpu_load_tlb(struct kvm_vcpu *vcpu, int cpu)
>
> static int kvm_vz_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> bool migrated, all;
>
> /*
> @@ -2704,7 +2704,7 @@ static int kvm_vz_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>
> static int kvm_vz_vcpu_put(struct kvm_vcpu *vcpu, int cpu)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>
> if (current->flags & PF_VCPU)
> kvm_vz_vcpu_save_wired(vcpu);
> @@ -3076,7 +3076,7 @@ static void kvm_vz_vcpu_uninit(struct kvm_vcpu *vcpu)
>
> static int kvm_vz_vcpu_setup(struct kvm_vcpu *vcpu)
> {
> - struct mips_coproc *cop0 = vcpu->arch.cop0;
> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> unsigned long count_hz = 100*1000*1000; /* default to 100 MHz */
>
> /*
> --
> 2.39.3
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MIPS: KVM: Fix NULL pointer dereference
2023-06-26 10:33 ` Paolo Bonzini
@ 2023-06-26 12:44 ` Huacai Chen
2023-06-29 0:07 ` Yu Zhao
2023-06-29 14:58 ` Jiaxun Yang
2 siblings, 0 replies; 9+ messages in thread
From: Huacai Chen @ 2023-06-26 12:44 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Huacai Chen, Thomas Bogendoerfer, linux-mips, kvm, Jiaxun Yang, stable
Hi, Paolo,
On Mon, Jun 26, 2023 at 6:33 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On Mon, Jun 26, 2023 at 9:59 AM Huacai Chen <chenhuacai@loongson.cn> wrote:
> >
> > After commit 45c7e8af4a5e3f0bea4ac209 ("MIPS: Remove KVM_TE support") we
> > get a NULL pointer dereference when creating a KVM guest:
>
> To be honest, a bug that needed 2 years to be reproduced is probably a
> sign that KVM/MIPS has no users. Any objections to removing it
> altogether?
I'm very sorry for that, in the past two years I have built MIPS
kernels with KVM daily, but only tested in QEMU TCG because I cross
compiled on a x86 machine. Meanwhile, Loongson distros are still using
the old LTS kernels so nearly no one sees the bug...
But anyway, I want to keep it at present. Sorry for that again.
Huacai
>
> Paolo
>
> >
> > [ 146.243409] Starting KVM with MIPS VZ extensions
> > [ 149.849151] CPU 3 Unable to handle kernel paging request at virtual address 0000000000000300, epc == ffffffffc06356ec, ra == ffffffffc063568c
> > [ 149.849177] Oops[#1]:
> > [ 149.849182] CPU: 3 PID: 2265 Comm: qemu-system-mip Not tainted 6.4.0-rc3+ #1671
> > [ 149.849188] Hardware name: THTF CX TL630 Series/THTF-LS3A4000-7A1000-ML4A, BIOS KL4.1F.TF.D.166.201225.R 12/25/2020
> > [ 149.849192] $ 0 : 0000000000000000 000000007400cce0 0000000000400004 ffffffff8119c740
> > [ 149.849209] $ 4 : 000000007400cce1 000000007400cce1 0000000000000000 0000000000000000
> > [ 149.849221] $ 8 : 000000240058bb36 ffffffff81421ac0 0000000000000000 0000000000400dc0
> > [ 149.849233] $12 : 9800000102a07cc8 ffffffff80e40e38 0000000000000001 0000000000400dc0
> > [ 149.849245] $16 : 0000000000000000 9800000106cd0000 9800000106cd0000 9800000100cce000
> > [ 149.849257] $20 : ffffffffc0632b28 ffffffffc05b31b0 9800000100ccca00 0000000000400000
> > [ 149.849269] $24 : 9800000106cd09ce ffffffff802f69d0
> > [ 149.849281] $28 : 9800000102a04000 9800000102a07cd0 98000001106a8000 ffffffffc063568c
> > [ 149.849293] Hi : 00000335b2111e66
> > [ 149.849295] Lo : 6668d90061ae0ae9
> > [ 149.849298] epc : ffffffffc06356ec kvm_vz_vcpu_setup+0xc4/0x328 [kvm]
> > [ 149.849324] ra : ffffffffc063568c kvm_vz_vcpu_setup+0x64/0x328 [kvm]
> > [ 149.849336] Status: 7400cce3 KX SX UX KERNEL EXL IE
> > [ 149.849351] Cause : 1000000c (ExcCode 03)
> > [ 149.849354] BadVA : 0000000000000300
> > [ 149.849357] PrId : 0014c004 (ICT Loongson-3)
> > [ 149.849360] Modules linked in: kvm nfnetlink_queue nfnetlink_log nfnetlink fuse sha256_generic libsha256 cfg80211 rfkill binfmt_misc vfat fat snd_hda_codec_hdmi input_leds led_class snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hda_core snd_pcm snd_timer snd serio_raw xhci_pci radeon drm_suballoc_helper drm_display_helper xhci_hcd ip_tables x_tables
> > [ 149.849432] Process qemu-system-mip (pid: 2265, threadinfo=00000000ae2982d2, task=0000000038e09ad4, tls=000000ffeba16030)
> > [ 149.849439] Stack : 9800000000000003 9800000100ccca00 9800000100ccc000 ffffffffc062cef4
> > [ 149.849453] 9800000102a07d18 c89b63a7ab338e00 0000000000000000 ffffffff811a0000
> > [ 149.849465] 0000000000000000 9800000106cd0000 ffffffff80e59938 98000001106a8920
> > [ 149.849476] ffffffff80e57f30 ffffffffc062854c ffffffff811a0000 9800000102bf4240
> > [ 149.849488] ffffffffc05b0000 ffffffff80e3a798 000000ff78000000 000000ff78000010
> > [ 149.849500] 0000000000000255 98000001021f7de0 98000001023f0078 ffffffff81434000
> > [ 149.849511] 0000000000000000 0000000000000000 9800000102ae0000 980000025e92ae28
> > [ 149.849523] 0000000000000000 c89b63a7ab338e00 0000000000000001 ffffffff8119dce0
> > [ 149.849535] 000000ff78000010 ffffffff804f3d3c 9800000102a07eb0 0000000000000255
> > [ 149.849546] 0000000000000000 ffffffff8049460c 000000ff78000010 0000000000000255
> > [ 149.849558] ...
> > [ 149.849565] Call Trace:
> > [ 149.849567] [<ffffffffc06356ec>] kvm_vz_vcpu_setup+0xc4/0x328 [kvm]
> > [ 149.849586] [<ffffffffc062cef4>] kvm_arch_vcpu_create+0x184/0x228 [kvm]
> > [ 149.849605] [<ffffffffc062854c>] kvm_vm_ioctl+0x64c/0xf28 [kvm]
> > [ 149.849623] [<ffffffff805209c0>] sys_ioctl+0xc8/0x118
> > [ 149.849631] [<ffffffff80219eb0>] syscall_common+0x34/0x58
> >
> > The root cause is the deletion of kvm_mips_commpage_init() leaves vcpu->
> > arch.cop0 NULL. So fix it by make cop0 from a pointer to an embed object.
> >
> > Fixes: 45c7e8af4a5e3f0bea4ac209 ("MIPS: Remove KVM_TE support")
> > Cc: stable@vger.kernel.org
> > Suggested-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > ---
> > arch/mips/include/asm/kvm_host.h | 6 +++---
> > arch/mips/kvm/emulate.c | 22 +++++++++++-----------
> > arch/mips/kvm/mips.c | 16 ++++++++--------
> > arch/mips/kvm/trace.h | 8 ++++----
> > arch/mips/kvm/vz.c | 20 ++++++++++----------
> > 5 files changed, 36 insertions(+), 36 deletions(-)
> >
> > diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
> > index 957121a495f0..04cedf9f8811 100644
> > --- a/arch/mips/include/asm/kvm_host.h
> > +++ b/arch/mips/include/asm/kvm_host.h
> > @@ -317,7 +317,7 @@ struct kvm_vcpu_arch {
> > unsigned int aux_inuse;
> >
> > /* COP0 State */
> > - struct mips_coproc *cop0;
> > + struct mips_coproc cop0;
> >
> > /* Resume PC after MMIO completion */
> > unsigned long io_pc;
> > @@ -698,7 +698,7 @@ static inline bool kvm_mips_guest_can_have_fpu(struct kvm_vcpu_arch *vcpu)
> > static inline bool kvm_mips_guest_has_fpu(struct kvm_vcpu_arch *vcpu)
> > {
> > return kvm_mips_guest_can_have_fpu(vcpu) &&
> > - kvm_read_c0_guest_config1(vcpu->cop0) & MIPS_CONF1_FP;
> > + kvm_read_c0_guest_config1(&vcpu->cop0) & MIPS_CONF1_FP;
> > }
> >
> > static inline bool kvm_mips_guest_can_have_msa(struct kvm_vcpu_arch *vcpu)
> > @@ -710,7 +710,7 @@ static inline bool kvm_mips_guest_can_have_msa(struct kvm_vcpu_arch *vcpu)
> > static inline bool kvm_mips_guest_has_msa(struct kvm_vcpu_arch *vcpu)
> > {
> > return kvm_mips_guest_can_have_msa(vcpu) &&
> > - kvm_read_c0_guest_config3(vcpu->cop0) & MIPS_CONF3_MSA;
> > + kvm_read_c0_guest_config3(&vcpu->cop0) & MIPS_CONF3_MSA;
> > }
> >
> > struct kvm_mips_callbacks {
> > diff --git a/arch/mips/kvm/emulate.c b/arch/mips/kvm/emulate.c
> > index edaec93a1a1f..e64372b8f66a 100644
> > --- a/arch/mips/kvm/emulate.c
> > +++ b/arch/mips/kvm/emulate.c
> > @@ -312,7 +312,7 @@ int kvm_get_badinstrp(u32 *opc, struct kvm_vcpu *vcpu, u32 *out)
> > */
> > int kvm_mips_count_disabled(struct kvm_vcpu *vcpu)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> >
> > return (vcpu->arch.count_ctl & KVM_REG_MIPS_COUNT_CTL_DC) ||
> > (kvm_read_c0_guest_cause(cop0) & CAUSEF_DC);
> > @@ -384,7 +384,7 @@ static inline ktime_t kvm_mips_count_time(struct kvm_vcpu *vcpu)
> > */
> > static u32 kvm_mips_read_count_running(struct kvm_vcpu *vcpu, ktime_t now)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> > ktime_t expires, threshold;
> > u32 count, compare;
> > int running;
> > @@ -444,7 +444,7 @@ static u32 kvm_mips_read_count_running(struct kvm_vcpu *vcpu, ktime_t now)
> > */
> > u32 kvm_mips_read_count(struct kvm_vcpu *vcpu)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> >
> > /* If count disabled just read static copy of count */
> > if (kvm_mips_count_disabled(vcpu))
> > @@ -502,7 +502,7 @@ ktime_t kvm_mips_freeze_hrtimer(struct kvm_vcpu *vcpu, u32 *count)
> > static void kvm_mips_resume_hrtimer(struct kvm_vcpu *vcpu,
> > ktime_t now, u32 count)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> > u32 compare;
> > u64 delta;
> > ktime_t expire;
> > @@ -603,7 +603,7 @@ int kvm_mips_restore_hrtimer(struct kvm_vcpu *vcpu, ktime_t before,
> > */
> > void kvm_mips_write_count(struct kvm_vcpu *vcpu, u32 count)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> > ktime_t now;
> >
> > /* Calculate bias */
> > @@ -649,7 +649,7 @@ void kvm_mips_init_count(struct kvm_vcpu *vcpu, unsigned long count_hz)
> > */
> > int kvm_mips_set_count_hz(struct kvm_vcpu *vcpu, s64 count_hz)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> > int dc;
> > ktime_t now;
> > u32 count;
> > @@ -696,7 +696,7 @@ int kvm_mips_set_count_hz(struct kvm_vcpu *vcpu, s64 count_hz)
> > */
> > void kvm_mips_write_compare(struct kvm_vcpu *vcpu, u32 compare, bool ack)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> > int dc;
> > u32 old_compare = kvm_read_c0_guest_compare(cop0);
> > s32 delta = compare - old_compare;
> > @@ -779,7 +779,7 @@ void kvm_mips_write_compare(struct kvm_vcpu *vcpu, u32 compare, bool ack)
> > */
> > static ktime_t kvm_mips_count_disable(struct kvm_vcpu *vcpu)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> > u32 count;
> > ktime_t now;
> >
> > @@ -806,7 +806,7 @@ static ktime_t kvm_mips_count_disable(struct kvm_vcpu *vcpu)
> > */
> > void kvm_mips_count_disable_cause(struct kvm_vcpu *vcpu)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> >
> > kvm_set_c0_guest_cause(cop0, CAUSEF_DC);
> > if (!(vcpu->arch.count_ctl & KVM_REG_MIPS_COUNT_CTL_DC))
> > @@ -826,7 +826,7 @@ void kvm_mips_count_disable_cause(struct kvm_vcpu *vcpu)
> > */
> > void kvm_mips_count_enable_cause(struct kvm_vcpu *vcpu)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> > u32 count;
> >
> > kvm_clear_c0_guest_cause(cop0, CAUSEF_DC);
> > @@ -852,7 +852,7 @@ void kvm_mips_count_enable_cause(struct kvm_vcpu *vcpu)
> > */
> > int kvm_mips_set_count_ctl(struct kvm_vcpu *vcpu, s64 count_ctl)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> > s64 changed = count_ctl ^ vcpu->arch.count_ctl;
> > s64 delta;
> > ktime_t expire, now;
> > diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> > index 884be4ef99dc..aa5583a7b05b 100644
> > --- a/arch/mips/kvm/mips.c
> > +++ b/arch/mips/kvm/mips.c
> > @@ -649,7 +649,7 @@ static int kvm_mips_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices)
> > static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
> > const struct kvm_one_reg *reg)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> > struct mips_fpu_struct *fpu = &vcpu->arch.fpu;
> > int ret;
> > s64 v;
> > @@ -761,7 +761,7 @@ static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
> > static int kvm_mips_set_reg(struct kvm_vcpu *vcpu,
> > const struct kvm_one_reg *reg)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> > struct mips_fpu_struct *fpu = &vcpu->arch.fpu;
> > s64 v;
> > s64 vs[2];
> > @@ -1086,7 +1086,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> > int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
> > {
> > return kvm_mips_pending_timer(vcpu) ||
> > - kvm_read_c0_guest_cause(vcpu->arch.cop0) & C_TI;
> > + kvm_read_c0_guest_cause(&vcpu->arch.cop0) & C_TI;
> > }
> >
> > int kvm_arch_vcpu_dump_regs(struct kvm_vcpu *vcpu)
> > @@ -1110,7 +1110,7 @@ int kvm_arch_vcpu_dump_regs(struct kvm_vcpu *vcpu)
> > kvm_debug("\thi: 0x%08lx\n", vcpu->arch.hi);
> > kvm_debug("\tlo: 0x%08lx\n", vcpu->arch.lo);
> >
> > - cop0 = vcpu->arch.cop0;
> > + cop0 = &vcpu->arch.cop0;
> > kvm_debug("\tStatus: 0x%08x, Cause: 0x%08x\n",
> > kvm_read_c0_guest_status(cop0),
> > kvm_read_c0_guest_cause(cop0));
> > @@ -1232,7 +1232,7 @@ static int __kvm_mips_handle_exit(struct kvm_vcpu *vcpu)
> >
> > case EXCCODE_TLBS:
> > kvm_debug("TLB ST fault: cause %#x, status %#x, PC: %p, BadVaddr: %#lx\n",
> > - cause, kvm_read_c0_guest_status(vcpu->arch.cop0), opc,
> > + cause, kvm_read_c0_guest_status(&vcpu->arch.cop0), opc,
> > badvaddr);
> >
> > ++vcpu->stat.tlbmiss_st_exits;
> > @@ -1304,7 +1304,7 @@ static int __kvm_mips_handle_exit(struct kvm_vcpu *vcpu)
> > kvm_get_badinstr(opc, vcpu, &inst);
> > kvm_err("Exception Code: %d, not yet handled, @ PC: %p, inst: 0x%08x BadVaddr: %#lx Status: %#x\n",
> > exccode, opc, inst, badvaddr,
> > - kvm_read_c0_guest_status(vcpu->arch.cop0));
> > + kvm_read_c0_guest_status(&vcpu->arch.cop0));
> > kvm_arch_vcpu_dump_regs(vcpu);
> > run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
> > ret = RESUME_HOST;
> > @@ -1377,7 +1377,7 @@ int noinstr kvm_mips_handle_exit(struct kvm_vcpu *vcpu)
> > /* Enable FPU for guest and restore context */
> > void kvm_own_fpu(struct kvm_vcpu *vcpu)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> > unsigned int sr, cfg5;
> >
> > preempt_disable();
> > @@ -1421,7 +1421,7 @@ void kvm_own_fpu(struct kvm_vcpu *vcpu)
> > /* Enable MSA for guest and restore context */
> > void kvm_own_msa(struct kvm_vcpu *vcpu)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> > unsigned int sr, cfg5;
> >
> > preempt_disable();
> > diff --git a/arch/mips/kvm/trace.h b/arch/mips/kvm/trace.h
> > index a8c7fd7bf6d2..136c3535a1cb 100644
> > --- a/arch/mips/kvm/trace.h
> > +++ b/arch/mips/kvm/trace.h
> > @@ -322,11 +322,11 @@ TRACE_EVENT_FN(kvm_guest_mode_change,
> > ),
> >
> > TP_fast_assign(
> > - __entry->epc = kvm_read_c0_guest_epc(vcpu->arch.cop0);
> > + __entry->epc = kvm_read_c0_guest_epc(&vcpu->arch.cop0);
> > __entry->pc = vcpu->arch.pc;
> > - __entry->badvaddr = kvm_read_c0_guest_badvaddr(vcpu->arch.cop0);
> > - __entry->status = kvm_read_c0_guest_status(vcpu->arch.cop0);
> > - __entry->cause = kvm_read_c0_guest_cause(vcpu->arch.cop0);
> > + __entry->badvaddr = kvm_read_c0_guest_badvaddr(&vcpu->arch.cop0);
> > + __entry->status = kvm_read_c0_guest_status(&vcpu->arch.cop0);
> > + __entry->cause = kvm_read_c0_guest_cause(&vcpu->arch.cop0);
> > ),
> >
> > TP_printk("EPC: 0x%08lx PC: 0x%08lx Status: 0x%08x Cause: 0x%08x BadVAddr: 0x%08lx",
> > diff --git a/arch/mips/kvm/vz.c b/arch/mips/kvm/vz.c
> > index 3d21cbfa7443..99d5a71e4300 100644
> > --- a/arch/mips/kvm/vz.c
> > +++ b/arch/mips/kvm/vz.c
> > @@ -422,7 +422,7 @@ static void _kvm_vz_restore_htimer(struct kvm_vcpu *vcpu,
> > */
> > static void kvm_vz_restore_timer(struct kvm_vcpu *vcpu)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> > u32 cause, compare;
> >
> > compare = kvm_read_sw_gc0_compare(cop0);
> > @@ -517,7 +517,7 @@ static void _kvm_vz_save_htimer(struct kvm_vcpu *vcpu,
> > */
> > static void kvm_vz_save_timer(struct kvm_vcpu *vcpu)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> > u32 gctl0, compare, cause;
> >
> > gctl0 = read_c0_guestctl0();
> > @@ -863,7 +863,7 @@ static unsigned long mips_process_maar(unsigned int op, unsigned long val)
> >
> > static void kvm_write_maari(struct kvm_vcpu *vcpu, unsigned long val)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> >
> > val &= MIPS_MAARI_INDEX;
> > if (val == MIPS_MAARI_INDEX)
> > @@ -876,7 +876,7 @@ static enum emulation_result kvm_vz_gpsi_cop0(union mips_instruction inst,
> > u32 *opc, u32 cause,
> > struct kvm_vcpu *vcpu)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> > enum emulation_result er = EMULATE_DONE;
> > u32 rt, rd, sel;
> > unsigned long curr_pc;
> > @@ -1911,7 +1911,7 @@ static int kvm_vz_get_one_reg(struct kvm_vcpu *vcpu,
> > const struct kvm_one_reg *reg,
> > s64 *v)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> > unsigned int idx;
> >
> > switch (reg->id) {
> > @@ -2081,7 +2081,7 @@ static int kvm_vz_get_one_reg(struct kvm_vcpu *vcpu,
> > case KVM_REG_MIPS_CP0_MAARI:
> > if (!cpu_guest_has_maar || cpu_guest_has_dyn_maar)
> > return -EINVAL;
> > - *v = kvm_read_sw_gc0_maari(vcpu->arch.cop0);
> > + *v = kvm_read_sw_gc0_maari(&vcpu->arch.cop0);
> > break;
> > #ifdef CONFIG_64BIT
> > case KVM_REG_MIPS_CP0_XCONTEXT:
> > @@ -2135,7 +2135,7 @@ static int kvm_vz_set_one_reg(struct kvm_vcpu *vcpu,
> > const struct kvm_one_reg *reg,
> > s64 v)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> > unsigned int idx;
> > int ret = 0;
> > unsigned int cur, change;
> > @@ -2562,7 +2562,7 @@ static void kvm_vz_vcpu_load_tlb(struct kvm_vcpu *vcpu, int cpu)
> >
> > static int kvm_vz_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> > bool migrated, all;
> >
> > /*
> > @@ -2704,7 +2704,7 @@ static int kvm_vz_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
> >
> > static int kvm_vz_vcpu_put(struct kvm_vcpu *vcpu, int cpu)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> >
> > if (current->flags & PF_VCPU)
> > kvm_vz_vcpu_save_wired(vcpu);
> > @@ -3076,7 +3076,7 @@ static void kvm_vz_vcpu_uninit(struct kvm_vcpu *vcpu)
> >
> > static int kvm_vz_vcpu_setup(struct kvm_vcpu *vcpu)
> > {
> > - struct mips_coproc *cop0 = vcpu->arch.cop0;
> > + struct mips_coproc *cop0 = &vcpu->arch.cop0;
> > unsigned long count_hz = 100*1000*1000; /* default to 100 MHz */
> >
> > /*
> > --
> > 2.39.3
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MIPS: KVM: Fix NULL pointer dereference
2023-06-26 10:28 ` Philippe Mathieu-Daudé
@ 2023-06-28 7:34 ` Huacai Chen
0 siblings, 0 replies; 9+ messages in thread
From: Huacai Chen @ 2023-06-28 7:34 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Huacai Chen, Thomas Bogendoerfer, Paolo Bonzini, linux-mips, kvm,
Jiaxun Yang, stable
Hi, Philippe,
On Mon, Jun 26, 2023 at 6:28 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> On 26/6/23 09:49, Huacai Chen wrote:
> > After commit 45c7e8af4a5e3f0bea4ac209 ("MIPS: Remove KVM_TE support") we
> > get a NULL pointer dereference when creating a KVM guest:
> >
> > [ 146.243409] Starting KVM with MIPS VZ extensions
> > [ 149.849151] CPU 3 Unable to handle kernel paging request at virtual address 0000000000000300, epc == ffffffffc06356ec, ra == ffffffffc063568c
> > [ 149.849177] Oops[#1]:
> > [ 149.849182] CPU: 3 PID: 2265 Comm: qemu-system-mip Not tainted 6.4.0-rc3+ #1671
> > [ 149.849188] Hardware name: THTF CX TL630 Series/THTF-LS3A4000-7A1000-ML4A, BIOS KL4.1F.TF.D.166.201225.R 12/25/2020
> > [ 149.849192] $ 0 : 0000000000000000 000000007400cce0 0000000000400004 ffffffff8119c740
> > [ 149.849209] $ 4 : 000000007400cce1 000000007400cce1 0000000000000000 0000000000000000
> > [ 149.849221] $ 8 : 000000240058bb36 ffffffff81421ac0 0000000000000000 0000000000400dc0
> > [ 149.849233] $12 : 9800000102a07cc8 ffffffff80e40e38 0000000000000001 0000000000400dc0
> > [ 149.849245] $16 : 0000000000000000 9800000106cd0000 9800000106cd0000 9800000100cce000
> > [ 149.849257] $20 : ffffffffc0632b28 ffffffffc05b31b0 9800000100ccca00 0000000000400000
> > [ 149.849269] $24 : 9800000106cd09ce ffffffff802f69d0
> > [ 149.849281] $28 : 9800000102a04000 9800000102a07cd0 98000001106a8000 ffffffffc063568c
> > [ 149.849293] Hi : 00000335b2111e66
> > [ 149.849295] Lo : 6668d90061ae0ae9
> > [ 149.849298] epc : ffffffffc06356ec kvm_vz_vcpu_setup+0xc4/0x328 [kvm]
> > [ 149.849324] ra : ffffffffc063568c kvm_vz_vcpu_setup+0x64/0x328 [kvm]
> > [ 149.849336] Status: 7400cce3 KX SX UX KERNEL EXL IE
> > [ 149.849351] Cause : 1000000c (ExcCode 03)
> > [ 149.849354] BadVA : 0000000000000300
> > [ 149.849357] PrId : 0014c004 (ICT Loongson-3)
> > [ 149.849360] Modules linked in: kvm nfnetlink_queue nfnetlink_log nfnetlink fuse sha256_generic libsha256 cfg80211 rfkill binfmt_misc vfat fat snd_hda_codec_hdmi input_leds led_class snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hda_core snd_pcm snd_timer snd serio_raw xhci_pci radeon drm_suballoc_helper drm_display_helper xhci_hcd ip_tables x_tables
> > [ 149.849432] Process qemu-system-mip (pid: 2265, threadinfo=00000000ae2982d2, task=0000000038e09ad4, tls=000000ffeba16030)
> > [ 149.849439] Stack : 9800000000000003 9800000100ccca00 9800000100ccc000 ffffffffc062cef4
> > [ 149.849453] 9800000102a07d18 c89b63a7ab338e00 0000000000000000 ffffffff811a0000
> > [ 149.849465] 0000000000000000 9800000106cd0000 ffffffff80e59938 98000001106a8920
> > [ 149.849476] ffffffff80e57f30 ffffffffc062854c ffffffff811a0000 9800000102bf4240
> > [ 149.849488] ffffffffc05b0000 ffffffff80e3a798 000000ff78000000 000000ff78000010
> > [ 149.849500] 0000000000000255 98000001021f7de0 98000001023f0078 ffffffff81434000
> > [ 149.849511] 0000000000000000 0000000000000000 9800000102ae0000 980000025e92ae28
> > [ 149.849523] 0000000000000000 c89b63a7ab338e00 0000000000000001 ffffffff8119dce0
> > [ 149.849535] 000000ff78000010 ffffffff804f3d3c 9800000102a07eb0 0000000000000255
> > [ 149.849546] 0000000000000000 ffffffff8049460c 000000ff78000010 0000000000000255
> > [ 149.849558] ...
> > [ 149.849565] Call Trace:
> > [ 149.849567] [<ffffffffc06356ec>] kvm_vz_vcpu_setup+0xc4/0x328 [kvm]
> > [ 149.849586] [<ffffffffc062cef4>] kvm_arch_vcpu_create+0x184/0x228 [kvm]
> > [ 149.849605] [<ffffffffc062854c>] kvm_vm_ioctl+0x64c/0xf28 [kvm]
> > [ 149.849623] [<ffffffff805209c0>] sys_ioctl+0xc8/0x118
> > [ 149.849631] [<ffffffff80219eb0>] syscall_common+0x34/0x58
> >
> > The root cause is the deletion of kvm_mips_commpage_init() leaves vcpu->
> > arch.cop0 NULL. So fix it by make cop0 from a pointer to an embed object.
>
> "by making ... to an embedded object."
OK, thanks.
>
> >
> > Fixes: 45c7e8af4a5e3f0bea4ac209 ("MIPS: Remove KVM_TE support")
> > Cc: stable@vger.kernel.org
>
> Reported-by: Yu Zhao <yuzhao@google.com>
OK, thanks.
>
> > Suggested-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > ---
> > arch/mips/include/asm/kvm_host.h | 6 +++---
> > arch/mips/kvm/emulate.c | 22 +++++++++++-----------
> > arch/mips/kvm/mips.c | 16 ++++++++--------
> > arch/mips/kvm/trace.h | 8 ++++----
> > arch/mips/kvm/vz.c | 20 ++++++++++----------
> > 5 files changed, 36 insertions(+), 36 deletions(-)
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
OK, thanks.
Huacai
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MIPS: KVM: Fix NULL pointer dereference
2023-06-26 10:33 ` Paolo Bonzini
2023-06-26 12:44 ` Huacai Chen
@ 2023-06-29 0:07 ` Yu Zhao
2023-06-29 6:24 ` Huacai Chen
2023-06-29 14:58 ` Jiaxun Yang
2 siblings, 1 reply; 9+ messages in thread
From: Yu Zhao @ 2023-06-29 0:07 UTC (permalink / raw)
To: pbonzini
Cc: chenhuacai, chenhuacai, jiaxun.yang, kvm, linux-mips, stable, tsbogend
On Mon, Jun 26, 2023 at 6:33 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On Mon, Jun 26, 2023 at 9:59 AM Huacai Chen <chenhuacai@loongson.cn> wrote:
> >
> > After commit 45c7e8af4a5e3f0bea4ac209 ("MIPS: Remove KVM_TE support") we
> > get a NULL pointer dereference when creating a KVM guest:
>
> To be honest, a bug that needed 2 years to be reproduced is probably a
> sign that KVM/MIPS has no users. Any objections to removing it
> altogether?
ACK:
1. It's still broken after this patch [1]. The most well-tested MIPS
distros, i.e., Debian/OpenWrt, have CONFIG_KVM=n. (The latter doesn't
even provide the QEMU package on MIPS.)
2. Burden on QEMU dev. There is no guarantee that QEMU would work with
KVM even if we could fix the kernel -- it actually does not until
v8.0 [1], which is by luck:
commit a844873512400fae6bed9e87694dc96ff2f15f39
Author: Paolo Bonzini <pbonzini@redhat.com>
Date: Sun Dec 18 01:06:45 2022 +0100
mips: Remove support for trap and emulate KVM
This support was limited to the Malta board, drop it.
I do not have a machine that can run VZ KVM, so I am assuming
that it works for -M malta as well.
(The latest Debian stable only ships v7.2.)
[1] https://lore.kernel.org/r/ZI0R76Fx25Q2EThZ@google.com/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MIPS: KVM: Fix NULL pointer dereference
2023-06-29 0:07 ` Yu Zhao
@ 2023-06-29 6:24 ` Huacai Chen
2023-06-29 8:54 ` Yu Zhao
0 siblings, 1 reply; 9+ messages in thread
From: Huacai Chen @ 2023-06-29 6:24 UTC (permalink / raw)
To: Yu Zhao
Cc: pbonzini, chenhuacai, jiaxun.yang, kvm, linux-mips, stable, tsbogend
Hi, Zhao,
On Thu, Jun 29, 2023 at 8:07 AM Yu Zhao <yuzhao@google.com> wrote:
>
> On Mon, Jun 26, 2023 at 6:33 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > On Mon, Jun 26, 2023 at 9:59 AM Huacai Chen <chenhuacai@loongson.cn> wrote:
> > >
> > > After commit 45c7e8af4a5e3f0bea4ac209 ("MIPS: Remove KVM_TE support") we
> > > get a NULL pointer dereference when creating a KVM guest:
> >
> > To be honest, a bug that needed 2 years to be reproduced is probably a
> > sign that KVM/MIPS has no users. Any objections to removing it
> > altogether?
>
> ACK:
> 1. It's still broken after this patch [1]. The most well-tested MIPS
> distros, i.e., Debian/OpenWrt, have CONFIG_KVM=n. (The latter doesn't
> even provide the QEMU package on MIPS.)
> 2. Burden on QEMU dev. There is no guarantee that QEMU would work with
> KVM even if we could fix the kernel -- it actually does not until
> v8.0 [1], which is by luck:
>
> commit a844873512400fae6bed9e87694dc96ff2f15f39
> Author: Paolo Bonzini <pbonzini@redhat.com>
> Date: Sun Dec 18 01:06:45 2022 +0100
>
> mips: Remove support for trap and emulate KVM
>
> This support was limited to the Malta board, drop it.
> I do not have a machine that can run VZ KVM, so I am assuming
> that it works for -M malta as well.
>
> (The latest Debian stable only ships v7.2.)
>
> [1] https://lore.kernel.org/r/ZI0R76Fx25Q2EThZ@google.com/
My testbed is Loongson-3A4000 host + Loongson-3A4000 guest + Qemu8.0,
both TCG and KVM works.
Some thoughts:
1, I think your host is malta, but you cannot use a malta host to boot
a Loongson guest, at least their kernels use different page sizes.
2, commit a844873512400fae6bed9e87694dc96f remove the TE KVM, so if
you are trying VZ KVM (but it seems you are using TE KVM), it can
break nothing.
Huacai
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MIPS: KVM: Fix NULL pointer dereference
2023-06-29 6:24 ` Huacai Chen
@ 2023-06-29 8:54 ` Yu Zhao
0 siblings, 0 replies; 9+ messages in thread
From: Yu Zhao @ 2023-06-29 8:54 UTC (permalink / raw)
To: Huacai Chen
Cc: pbonzini, chenhuacai, jiaxun.yang, kvm, linux-mips, stable, tsbogend
On Thu, Jun 29, 2023 at 12:24 AM Huacai Chen <chenhuacai@kernel.org> wrote:
>
> Hi, Zhao,
>
> On Thu, Jun 29, 2023 at 8:07 AM Yu Zhao <yuzhao@google.com> wrote:
> >
> > On Mon, Jun 26, 2023 at 6:33 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> > >
> > > On Mon, Jun 26, 2023 at 9:59 AM Huacai Chen <chenhuacai@loongson.cn> wrote:
> > > >
> > > > After commit 45c7e8af4a5e3f0bea4ac209 ("MIPS: Remove KVM_TE support") we
> > > > get a NULL pointer dereference when creating a KVM guest:
> > >
> > > To be honest, a bug that needed 2 years to be reproduced is probably a
> > > sign that KVM/MIPS has no users. Any objections to removing it
> > > altogether?
> >
> > ACK:
> > 1. It's still broken after this patch [1]. The most well-tested MIPS
> > distros, i.e., Debian/OpenWrt, have CONFIG_KVM=n. (The latter doesn't
> > even provide the QEMU package on MIPS.)
> > 2. Burden on QEMU dev. There is no guarantee that QEMU would work with
> > KVM even if we could fix the kernel -- it actually does not until
> > v8.0 [1], which is by luck:
> >
> > commit a844873512400fae6bed9e87694dc96ff2f15f39
> > Author: Paolo Bonzini <pbonzini@redhat.com>
> > Date: Sun Dec 18 01:06:45 2022 +0100
> >
> > mips: Remove support for trap and emulate KVM
> >
> > This support was limited to the Malta board, drop it.
> > I do not have a machine that can run VZ KVM, so I am assuming
> > that it works for -M malta as well.
> >
> > (The latest Debian stable only ships v7.2.)
> >
> > [1] https://lore.kernel.org/r/ZI0R76Fx25Q2EThZ@google.com/
> My testbed is Loongson-3A4000 host + Loongson-3A4000 guest + Qemu8.0,
> both TCG and KVM works.
I tried TCG too, as reported in the link above, and had no luck.
> Some thoughts:
> 1, I think your host is malta, but you cannot use a malta host to boot
> a Loongson guest, at least their kernels use different page sizes.
Good to know. Thanks.
> 2, commit a844873512400fae6bed9e87694dc96f remove the TE KVM, so if
> you are trying VZ KVM (but it seems you are using TE KVM), it can
> break nothing.
I fully understand :) I was trying to point out that QEMU/KVM had been
broken for over 2 years -- neither TE (removed from KVM) nor VZ works
with 7.2, which is probably why you used 8.0 too -- until that commit
*accidentally* fixed VZ (it was supposed to be a cleanup, not a fix).
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MIPS: KVM: Fix NULL pointer dereference
2023-06-26 10:33 ` Paolo Bonzini
2023-06-26 12:44 ` Huacai Chen
2023-06-29 0:07 ` Yu Zhao
@ 2023-06-29 14:58 ` Jiaxun Yang
2 siblings, 0 replies; 9+ messages in thread
From: Jiaxun Yang @ 2023-06-29 14:58 UTC (permalink / raw)
To: Paolo Bonzini, Huacai Chen
Cc: Thomas Bogendoerfer, Huacai Chen, linux-mips, kvm, stable
在2023年6月26日六月 下午6:33，Paolo Bonzini写道：
> On Mon, Jun 26, 2023 at 9:59 AM Huacai Chen <chenhuacai@loongson.cn> wrote:
>>
>> After commit 45c7e8af4a5e3f0bea4ac209 ("MIPS: Remove KVM_TE support") we
>> get a NULL pointer dereference when creating a KVM guest:
>
> To be honest, a bug that needed 2 years to be reproduced is probably a
> sign that KVM/MIPS has no users. Any objections to removing it
> altogether?
I am frequently doing boot test with QEMU & using MIPS KVM to test kernel on Loongson.
Though I haven't upgrade host kernel for a while, that's why the issue wasn't caught by me.
Also AFAIK there are several Loongson cloud service still using KVM.
So I don't want to see it go.
Thanks
Jiaxun
>
> Paolo
>
>>
>> [ 146.243409] Starting KVM with MIPS VZ extensions
>> [ 149.849151] CPU 3 Unable to handle kernel paging request at virtual address 0000000000000300, epc == ffffffffc06356ec, ra == ffffffffc063568c
>> [ 149.849177] Oops[#1]:
>> [ 149.849182] CPU: 3 PID: 2265 Comm: qemu-system-mip Not tainted 6.4.0-rc3+ #1671
>> [ 149.849188] Hardware name: THTF CX TL630 Series/THTF-LS3A4000-7A1000-ML4A, BIOS KL4.1F.TF.D.166.201225.R 12/25/2020
>> [ 149.849192] $ 0 : 0000000000000000 000000007400cce0 0000000000400004 ffffffff8119c740
>> [ 149.849209] $ 4 : 000000007400cce1 000000007400cce1 0000000000000000 0000000000000000
>> [ 149.849221] $ 8 : 000000240058bb36 ffffffff81421ac0 0000000000000000 0000000000400dc0
>> [ 149.849233] $12 : 9800000102a07cc8 ffffffff80e40e38 0000000000000001 0000000000400dc0
>> [ 149.849245] $16 : 0000000000000000 9800000106cd0000 9800000106cd0000 9800000100cce000
>> [ 149.849257] $20 : ffffffffc0632b28 ffffffffc05b31b0 9800000100ccca00 0000000000400000
>> [ 149.849269] $24 : 9800000106cd09ce ffffffff802f69d0
>> [ 149.849281] $28 : 9800000102a04000 9800000102a07cd0 98000001106a8000 ffffffffc063568c
>> [ 149.849293] Hi : 00000335b2111e66
>> [ 149.849295] Lo : 6668d90061ae0ae9
>> [ 149.849298] epc : ffffffffc06356ec kvm_vz_vcpu_setup+0xc4/0x328 [kvm]
>> [ 149.849324] ra : ffffffffc063568c kvm_vz_vcpu_setup+0x64/0x328 [kvm]
>> [ 149.849336] Status: 7400cce3 KX SX UX KERNEL EXL IE
>> [ 149.849351] Cause : 1000000c (ExcCode 03)
>> [ 149.849354] BadVA : 0000000000000300
>> [ 149.849357] PrId : 0014c004 (ICT Loongson-3)
>> [ 149.849360] Modules linked in: kvm nfnetlink_queue nfnetlink_log nfnetlink fuse sha256_generic libsha256 cfg80211 rfkill binfmt_misc vfat fat snd_hda_codec_hdmi input_leds led_class snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hda_core snd_pcm snd_timer snd serio_raw xhci_pci radeon drm_suballoc_helper drm_display_helper xhci_hcd ip_tables x_tables
>> [ 149.849432] Process qemu-system-mip (pid: 2265, threadinfo=00000000ae2982d2, task=0000000038e09ad4, tls=000000ffeba16030)
>> [ 149.849439] Stack : 9800000000000003 9800000100ccca00 9800000100ccc000 ffffffffc062cef4
>> [ 149.849453] 9800000102a07d18 c89b63a7ab338e00 0000000000000000 ffffffff811a0000
>> [ 149.849465] 0000000000000000 9800000106cd0000 ffffffff80e59938 98000001106a8920
>> [ 149.849476] ffffffff80e57f30 ffffffffc062854c ffffffff811a0000 9800000102bf4240
>> [ 149.849488] ffffffffc05b0000 ffffffff80e3a798 000000ff78000000 000000ff78000010
>> [ 149.849500] 0000000000000255 98000001021f7de0 98000001023f0078 ffffffff81434000
>> [ 149.849511] 0000000000000000 0000000000000000 9800000102ae0000 980000025e92ae28
>> [ 149.849523] 0000000000000000 c89b63a7ab338e00 0000000000000001 ffffffff8119dce0
>> [ 149.849535] 000000ff78000010 ffffffff804f3d3c 9800000102a07eb0 0000000000000255
>> [ 149.849546] 0000000000000000 ffffffff8049460c 000000ff78000010 0000000000000255
>> [ 149.849558] ...
>> [ 149.849565] Call Trace:
>> [ 149.849567] [<ffffffffc06356ec>] kvm_vz_vcpu_setup+0xc4/0x328 [kvm]
>> [ 149.849586] [<ffffffffc062cef4>] kvm_arch_vcpu_create+0x184/0x228 [kvm]
>> [ 149.849605] [<ffffffffc062854c>] kvm_vm_ioctl+0x64c/0xf28 [kvm]
>> [ 149.849623] [<ffffffff805209c0>] sys_ioctl+0xc8/0x118
>> [ 149.849631] [<ffffffff80219eb0>] syscall_common+0x34/0x58
>>
>> The root cause is the deletion of kvm_mips_commpage_init() leaves vcpu->
>> arch.cop0 NULL. So fix it by make cop0 from a pointer to an embed object.
>>
>> Fixes: 45c7e8af4a5e3f0bea4ac209 ("MIPS: Remove KVM_TE support")
>> Cc: stable@vger.kernel.org
>> Suggested-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
>> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
>> ---
>> arch/mips/include/asm/kvm_host.h | 6 +++---
>> arch/mips/kvm/emulate.c | 22 +++++++++++-----------
>> arch/mips/kvm/mips.c | 16 ++++++++--------
>> arch/mips/kvm/trace.h | 8 ++++----
>> arch/mips/kvm/vz.c | 20 ++++++++++----------
>> 5 files changed, 36 insertions(+), 36 deletions(-)
>>
>> diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
>> index 957121a495f0..04cedf9f8811 100644
>> --- a/arch/mips/include/asm/kvm_host.h
>> +++ b/arch/mips/include/asm/kvm_host.h
>> @@ -317,7 +317,7 @@ struct kvm_vcpu_arch {
>> unsigned int aux_inuse;
>>
>> /* COP0 State */
>> - struct mips_coproc *cop0;
>> + struct mips_coproc cop0;
>>
>> /* Resume PC after MMIO completion */
>> unsigned long io_pc;
>> @@ -698,7 +698,7 @@ static inline bool kvm_mips_guest_can_have_fpu(struct kvm_vcpu_arch *vcpu)
>> static inline bool kvm_mips_guest_has_fpu(struct kvm_vcpu_arch *vcpu)
>> {
>> return kvm_mips_guest_can_have_fpu(vcpu) &&
>> - kvm_read_c0_guest_config1(vcpu->cop0) & MIPS_CONF1_FP;
>> + kvm_read_c0_guest_config1(&vcpu->cop0) & MIPS_CONF1_FP;
>> }
>>
>> static inline bool kvm_mips_guest_can_have_msa(struct kvm_vcpu_arch *vcpu)
>> @@ -710,7 +710,7 @@ static inline bool kvm_mips_guest_can_have_msa(struct kvm_vcpu_arch *vcpu)
>> static inline bool kvm_mips_guest_has_msa(struct kvm_vcpu_arch *vcpu)
>> {
>> return kvm_mips_guest_can_have_msa(vcpu) &&
>> - kvm_read_c0_guest_config3(vcpu->cop0) & MIPS_CONF3_MSA;
>> + kvm_read_c0_guest_config3(&vcpu->cop0) & MIPS_CONF3_MSA;
>> }
>>
>> struct kvm_mips_callbacks {
>> diff --git a/arch/mips/kvm/emulate.c b/arch/mips/kvm/emulate.c
>> index edaec93a1a1f..e64372b8f66a 100644
>> --- a/arch/mips/kvm/emulate.c
>> +++ b/arch/mips/kvm/emulate.c
>> @@ -312,7 +312,7 @@ int kvm_get_badinstrp(u32 *opc, struct kvm_vcpu *vcpu, u32 *out)
>> */
>> int kvm_mips_count_disabled(struct kvm_vcpu *vcpu)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>>
>> return (vcpu->arch.count_ctl & KVM_REG_MIPS_COUNT_CTL_DC) ||
>> (kvm_read_c0_guest_cause(cop0) & CAUSEF_DC);
>> @@ -384,7 +384,7 @@ static inline ktime_t kvm_mips_count_time(struct kvm_vcpu *vcpu)
>> */
>> static u32 kvm_mips_read_count_running(struct kvm_vcpu *vcpu, ktime_t now)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>> ktime_t expires, threshold;
>> u32 count, compare;
>> int running;
>> @@ -444,7 +444,7 @@ static u32 kvm_mips_read_count_running(struct kvm_vcpu *vcpu, ktime_t now)
>> */
>> u32 kvm_mips_read_count(struct kvm_vcpu *vcpu)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>>
>> /* If count disabled just read static copy of count */
>> if (kvm_mips_count_disabled(vcpu))
>> @@ -502,7 +502,7 @@ ktime_t kvm_mips_freeze_hrtimer(struct kvm_vcpu *vcpu, u32 *count)
>> static void kvm_mips_resume_hrtimer(struct kvm_vcpu *vcpu,
>> ktime_t now, u32 count)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>> u32 compare;
>> u64 delta;
>> ktime_t expire;
>> @@ -603,7 +603,7 @@ int kvm_mips_restore_hrtimer(struct kvm_vcpu *vcpu, ktime_t before,
>> */
>> void kvm_mips_write_count(struct kvm_vcpu *vcpu, u32 count)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>> ktime_t now;
>>
>> /* Calculate bias */
>> @@ -649,7 +649,7 @@ void kvm_mips_init_count(struct kvm_vcpu *vcpu, unsigned long count_hz)
>> */
>> int kvm_mips_set_count_hz(struct kvm_vcpu *vcpu, s64 count_hz)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>> int dc;
>> ktime_t now;
>> u32 count;
>> @@ -696,7 +696,7 @@ int kvm_mips_set_count_hz(struct kvm_vcpu *vcpu, s64 count_hz)
>> */
>> void kvm_mips_write_compare(struct kvm_vcpu *vcpu, u32 compare, bool ack)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>> int dc;
>> u32 old_compare = kvm_read_c0_guest_compare(cop0);
>> s32 delta = compare - old_compare;
>> @@ -779,7 +779,7 @@ void kvm_mips_write_compare(struct kvm_vcpu *vcpu, u32 compare, bool ack)
>> */
>> static ktime_t kvm_mips_count_disable(struct kvm_vcpu *vcpu)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>> u32 count;
>> ktime_t now;
>>
>> @@ -806,7 +806,7 @@ static ktime_t kvm_mips_count_disable(struct kvm_vcpu *vcpu)
>> */
>> void kvm_mips_count_disable_cause(struct kvm_vcpu *vcpu)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>>
>> kvm_set_c0_guest_cause(cop0, CAUSEF_DC);
>> if (!(vcpu->arch.count_ctl & KVM_REG_MIPS_COUNT_CTL_DC))
>> @@ -826,7 +826,7 @@ void kvm_mips_count_disable_cause(struct kvm_vcpu *vcpu)
>> */
>> void kvm_mips_count_enable_cause(struct kvm_vcpu *vcpu)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>> u32 count;
>>
>> kvm_clear_c0_guest_cause(cop0, CAUSEF_DC);
>> @@ -852,7 +852,7 @@ void kvm_mips_count_enable_cause(struct kvm_vcpu *vcpu)
>> */
>> int kvm_mips_set_count_ctl(struct kvm_vcpu *vcpu, s64 count_ctl)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>> s64 changed = count_ctl ^ vcpu->arch.count_ctl;
>> s64 delta;
>> ktime_t expire, now;
>> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
>> index 884be4ef99dc..aa5583a7b05b 100644
>> --- a/arch/mips/kvm/mips.c
>> +++ b/arch/mips/kvm/mips.c
>> @@ -649,7 +649,7 @@ static int kvm_mips_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices)
>> static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
>> const struct kvm_one_reg *reg)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>> struct mips_fpu_struct *fpu = &vcpu->arch.fpu;
>> int ret;
>> s64 v;
>> @@ -761,7 +761,7 @@ static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
>> static int kvm_mips_set_reg(struct kvm_vcpu *vcpu,
>> const struct kvm_one_reg *reg)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>> struct mips_fpu_struct *fpu = &vcpu->arch.fpu;
>> s64 v;
>> s64 vs[2];
>> @@ -1086,7 +1086,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>> int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
>> {
>> return kvm_mips_pending_timer(vcpu) ||
>> - kvm_read_c0_guest_cause(vcpu->arch.cop0) & C_TI;
>> + kvm_read_c0_guest_cause(&vcpu->arch.cop0) & C_TI;
>> }
>>
>> int kvm_arch_vcpu_dump_regs(struct kvm_vcpu *vcpu)
>> @@ -1110,7 +1110,7 @@ int kvm_arch_vcpu_dump_regs(struct kvm_vcpu *vcpu)
>> kvm_debug("\thi: 0x%08lx\n", vcpu->arch.hi);
>> kvm_debug("\tlo: 0x%08lx\n", vcpu->arch.lo);
>>
>> - cop0 = vcpu->arch.cop0;
>> + cop0 = &vcpu->arch.cop0;
>> kvm_debug("\tStatus: 0x%08x, Cause: 0x%08x\n",
>> kvm_read_c0_guest_status(cop0),
>> kvm_read_c0_guest_cause(cop0));
>> @@ -1232,7 +1232,7 @@ static int __kvm_mips_handle_exit(struct kvm_vcpu *vcpu)
>>
>> case EXCCODE_TLBS:
>> kvm_debug("TLB ST fault: cause %#x, status %#x, PC: %p, BadVaddr: %#lx\n",
>> - cause, kvm_read_c0_guest_status(vcpu->arch.cop0), opc,
>> + cause, kvm_read_c0_guest_status(&vcpu->arch.cop0), opc,
>> badvaddr);
>>
>> ++vcpu->stat.tlbmiss_st_exits;
>> @@ -1304,7 +1304,7 @@ static int __kvm_mips_handle_exit(struct kvm_vcpu *vcpu)
>> kvm_get_badinstr(opc, vcpu, &inst);
>> kvm_err("Exception Code: %d, not yet handled, @ PC: %p, inst: 0x%08x BadVaddr: %#lx Status: %#x\n",
>> exccode, opc, inst, badvaddr,
>> - kvm_read_c0_guest_status(vcpu->arch.cop0));
>> + kvm_read_c0_guest_status(&vcpu->arch.cop0));
>> kvm_arch_vcpu_dump_regs(vcpu);
>> run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
>> ret = RESUME_HOST;
>> @@ -1377,7 +1377,7 @@ int noinstr kvm_mips_handle_exit(struct kvm_vcpu *vcpu)
>> /* Enable FPU for guest and restore context */
>> void kvm_own_fpu(struct kvm_vcpu *vcpu)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>> unsigned int sr, cfg5;
>>
>> preempt_disable();
>> @@ -1421,7 +1421,7 @@ void kvm_own_fpu(struct kvm_vcpu *vcpu)
>> /* Enable MSA for guest and restore context */
>> void kvm_own_msa(struct kvm_vcpu *vcpu)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>> unsigned int sr, cfg5;
>>
>> preempt_disable();
>> diff --git a/arch/mips/kvm/trace.h b/arch/mips/kvm/trace.h
>> index a8c7fd7bf6d2..136c3535a1cb 100644
>> --- a/arch/mips/kvm/trace.h
>> +++ b/arch/mips/kvm/trace.h
>> @@ -322,11 +322,11 @@ TRACE_EVENT_FN(kvm_guest_mode_change,
>> ),
>>
>> TP_fast_assign(
>> - __entry->epc = kvm_read_c0_guest_epc(vcpu->arch.cop0);
>> + __entry->epc = kvm_read_c0_guest_epc(&vcpu->arch.cop0);
>> __entry->pc = vcpu->arch.pc;
>> - __entry->badvaddr = kvm_read_c0_guest_badvaddr(vcpu->arch.cop0);
>> - __entry->status = kvm_read_c0_guest_status(vcpu->arch.cop0);
>> - __entry->cause = kvm_read_c0_guest_cause(vcpu->arch.cop0);
>> + __entry->badvaddr = kvm_read_c0_guest_badvaddr(&vcpu->arch.cop0);
>> + __entry->status = kvm_read_c0_guest_status(&vcpu->arch.cop0);
>> + __entry->cause = kvm_read_c0_guest_cause(&vcpu->arch.cop0);
>> ),
>>
>> TP_printk("EPC: 0x%08lx PC: 0x%08lx Status: 0x%08x Cause: 0x%08x BadVAddr: 0x%08lx",
>> diff --git a/arch/mips/kvm/vz.c b/arch/mips/kvm/vz.c
>> index 3d21cbfa7443..99d5a71e4300 100644
>> --- a/arch/mips/kvm/vz.c
>> +++ b/arch/mips/kvm/vz.c
>> @@ -422,7 +422,7 @@ static void _kvm_vz_restore_htimer(struct kvm_vcpu *vcpu,
>> */
>> static void kvm_vz_restore_timer(struct kvm_vcpu *vcpu)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>> u32 cause, compare;
>>
>> compare = kvm_read_sw_gc0_compare(cop0);
>> @@ -517,7 +517,7 @@ static void _kvm_vz_save_htimer(struct kvm_vcpu *vcpu,
>> */
>> static void kvm_vz_save_timer(struct kvm_vcpu *vcpu)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>> u32 gctl0, compare, cause;
>>
>> gctl0 = read_c0_guestctl0();
>> @@ -863,7 +863,7 @@ static unsigned long mips_process_maar(unsigned int op, unsigned long val)
>>
>> static void kvm_write_maari(struct kvm_vcpu *vcpu, unsigned long val)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>>
>> val &= MIPS_MAARI_INDEX;
>> if (val == MIPS_MAARI_INDEX)
>> @@ -876,7 +876,7 @@ static enum emulation_result kvm_vz_gpsi_cop0(union mips_instruction inst,
>> u32 *opc, u32 cause,
>> struct kvm_vcpu *vcpu)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>> enum emulation_result er = EMULATE_DONE;
>> u32 rt, rd, sel;
>> unsigned long curr_pc;
>> @@ -1911,7 +1911,7 @@ static int kvm_vz_get_one_reg(struct kvm_vcpu *vcpu,
>> const struct kvm_one_reg *reg,
>> s64 *v)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>> unsigned int idx;
>>
>> switch (reg->id) {
>> @@ -2081,7 +2081,7 @@ static int kvm_vz_get_one_reg(struct kvm_vcpu *vcpu,
>> case KVM_REG_MIPS_CP0_MAARI:
>> if (!cpu_guest_has_maar || cpu_guest_has_dyn_maar)
>> return -EINVAL;
>> - *v = kvm_read_sw_gc0_maari(vcpu->arch.cop0);
>> + *v = kvm_read_sw_gc0_maari(&vcpu->arch.cop0);
>> break;
>> #ifdef CONFIG_64BIT
>> case KVM_REG_MIPS_CP0_XCONTEXT:
>> @@ -2135,7 +2135,7 @@ static int kvm_vz_set_one_reg(struct kvm_vcpu *vcpu,
>> const struct kvm_one_reg *reg,
>> s64 v)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>> unsigned int idx;
>> int ret = 0;
>> unsigned int cur, change;
>> @@ -2562,7 +2562,7 @@ static void kvm_vz_vcpu_load_tlb(struct kvm_vcpu *vcpu, int cpu)
>>
>> static int kvm_vz_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>> bool migrated, all;
>>
>> /*
>> @@ -2704,7 +2704,7 @@ static int kvm_vz_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>>
>> static int kvm_vz_vcpu_put(struct kvm_vcpu *vcpu, int cpu)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>>
>> if (current->flags & PF_VCPU)
>> kvm_vz_vcpu_save_wired(vcpu);
>> @@ -3076,7 +3076,7 @@ static void kvm_vz_vcpu_uninit(struct kvm_vcpu *vcpu)
>>
>> static int kvm_vz_vcpu_setup(struct kvm_vcpu *vcpu)
>> {
>> - struct mips_coproc *cop0 = vcpu->arch.cop0;
>> + struct mips_coproc *cop0 = &vcpu->arch.cop0;
>> unsigned long count_hz = 100*1000*1000; /* default to 100 MHz */
>>
>> /*
>> --
>> 2.39.3
>>
--
- Jiaxun
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-06-29 14:59 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-26 7:49 [PATCH] MIPS: KVM: Fix NULL pointer dereference Huacai Chen
2023-06-26 10:28 ` Philippe Mathieu-Daudé
2023-06-28 7:34 ` Huacai Chen
2023-06-26 10:33 ` Paolo Bonzini
2023-06-26 12:44 ` Huacai Chen
2023-06-29 0:07 ` Yu Zhao
2023-06-29 6:24 ` Huacai Chen
2023-06-29 8:54 ` Yu Zhao
2023-06-29 14:58 ` Jiaxun Yang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).