[PATCH v4 0/4] LoongArch: KVM: Harden interrupt injection

[PATCH v4 0/4] LoongArch: KVM: Harden interrupt injection

[Bibo Mao]

API kvm_vcpu_ioctl_interrupt() is mainly to used to deliver interrupt
from user mode, and internal APIs kvm_queue_irq() and kvm_dequeue_irq()
are used in kernel mode, also moves IPI interrupt handling within lock
protection.

Also this patch replaces kvm_err() with WARN_ONCE() to avoid noise
kernel log.

---
v3 ... v4:
  1. From AI assist, replace kvm_err() with kvm_pr_unimpl() rather than
     WARN_ONCE(), since host may crash with WARN_ONCE() when kernel
     parameter panic_on_warn=1 is set.
  2. Add one new patch 3 which returns directly when IPI address is not
     aligned, rather than only print WARN_ON_ONCE() and continue to run.

v2 ... v3:
  1. Split the patches into three smaller ones, the first patch removes
     old default case and kvm_err() since it is impossible to happen.
     And the second patch replaces kvm_err() with WARN_ONCE().

v1 ... v2:
  1. Add border check with ipnum in eiointc_set_sw_coreisr() and
     eiointc_update_irq(), so that injected interrupt vector is valid.

  2. Move IPI inject and ack within lock to avoid contention in
     ipi_set() and ipi_clear().

  3. Add kvm_arch_irqchip_in_kernel() check in user mode irq injection
    in function kvm_vcpu_ioctl_interrupt(), contention of user mode irq
    injection is assured from user mode VMM.
---
Bibo Mao (4):
  LoongArch: KVM: Set default MAILBOX access size with 8
  LoongArch: KVM: Replace kvm_err() with kvm_pr_unimpl()
  LoongArch: KVM: Return directly when IPI address is not aligned
  LoongArch: KVM: Use internal API to deliver interrupt in kernel mode

 arch/loongarch/kvm/exit.c         | 16 +++----
 arch/loongarch/kvm/intc/dmsintc.c | 15 ++++---
 arch/loongarch/kvm/intc/eiointc.c | 32 +++++++------
 arch/loongarch/kvm/intc/ipi.c     | 75 +++++++++++++------------------
 arch/loongarch/kvm/intc/pch_pic.c | 12 ++---
 arch/loongarch/kvm/vcpu.c         |  5 ++-
 6 files changed, 76 insertions(+), 79 deletions(-)


base-commit: 0e35b9b6ec0ffcc5e23cbdec09f5c622ad532b53
-- 
2.39.3

[PATCH v4 1/4] LoongArch: KVM: Set default MAILBOX access size with 8

[Bibo Mao]

With IOCSR instruction emulation in function kvm_emu_iocsr(), the possible
size combination is 1/2/4/8 and no other combinations. Here remove the
old default case and replace the default case with 8 in read_mailbox()
and write_mailbox().

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 arch/loongarch/kvm/intc/ipi.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/loongarch/kvm/intc/ipi.c b/arch/loongarch/kvm/intc/ipi.c
index 4fa0897d7bdb..d70bf415c7fc 100644
--- a/arch/loongarch/kvm/intc/ipi.c
+++ b/arch/loongarch/kvm/intc/ipi.c
@@ -67,11 +67,8 @@ static uint64_t read_mailbox(struct kvm_vcpu *vcpu, int offset, int len)
 		return data & 0xffff;
 	case 4:
 		return data & 0xffffffff;
-	case 8:
-		return data;
 	default:
-		kvm_err("%s: unknown data len: %d\n", __func__, len);
-		return 0;
+		return data;
 	}
 }
 
@@ -92,11 +89,9 @@ static void write_mailbox(struct kvm_vcpu *vcpu, int offset, uint64_t data, int
 	case 4:
 		*(unsigned int *)pbuf = (unsigned int)data;
 		break;
-	case 8:
+	default:
 		*(unsigned long *)pbuf = (unsigned long)data;
 		break;
-	default:
-		kvm_err("%s: unknown data len: %d\n", __func__, len);
 	}
 	spin_unlock(&vcpu->arch.ipi_state.lock);
 }
-- 
2.39.3

Re: [PATCH v4 1/4] LoongArch: KVM: Set default MAILBOX access size with 8

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Guest-triggerable `WARN_ON_ONCE` via unaligned IOCSR address can crash the host, violating KVM guidelines.
- [High] `read_mailbox()` unconditionally performs an 8-byte read, leading to structural out-of-bounds array access and undefined behavior.
- [High] Read-modify-write cycles on IPI mailboxes are not protected by `ipi_state.lock`, leading to lost mailbox updates.
--

--- Patch [1]: [PATCH v4 1/4] LoongArch: KVM: Set default MAILBOX access size with 8 ---
commit ddb469fb4110b13e360da0bda699a489c854b92b
Author: Bibo Mao <maobibo@loongson.cn>

LoongArch: KVM: Set default MAILBOX access size with 8

This commit changes the default mailbox access size to 8 bytes in
read_mailbox() and write_mailbox() to handle IOCSR instruction emulation,
removing the previous error logging for unknown sizes.

> diff --git a/arch/loongarch/kvm/intc/ipi.c b/arch/loongarch/kvm/intc/ipi.c
> --- a/arch/loongarch/kvm/intc/ipi.c
> +++ b/arch/loongarch/kvm/intc/ipi.c
> @@ -67,11 +67,8 @@ static uint64_t read_mailbox(struct kvm_vcpu *vcpu, int offset, int len)
>  		return data & 0xffff;
>  	case 4:
>  		return data & 0xffffffff;
> -	case 8:
> -		return data;
>  	default:
> -		kvm_err("%s: unknown data len: %d\n", __func__, len);
> -		return 0;
> +		return data;
>  	}
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does the unconditional 8-byte
read in read_mailbox() risk an out-of-bounds array access?

Looking at read_mailbox(), it computes a byte pointer into the 32-byte
ipi_state.buf array based on the offset, and unconditionally casts it to an
unsigned long pointer to read 8 bytes before evaluating the size cases:

    data = *(ulong *)((void *)vcpu->arch.ipi_state.buf + (offset - 0x20));

If a guest performs a valid 4-byte read at the maximum valid offset, the
function still reads 8 bytes. This means the read extends beyond
ipi_state.buf and bleeds into adjacent struct members. Additionally,
unconditionally reading an 8-byte integer from a potentially unaligned
pointer might result in undefined behavior.

[ ... ]

> @@ -92,11 +89,9 @@ static void write_mailbox(struct kvm_vcpu *vcpu, int offset, uint64_t data, int
>  	case 4:
>  		*(unsigned int *)pbuf = (unsigned int)data;
>  		break;
> -	case 8:
> +	default:
>  		*(unsigned long *)pbuf = (unsigned long)data;
>  		break;
> -	default:
> -		kvm_err("%s: unknown data len: %d\n", __func__, len);
>  	}
>  	spin_unlock(&vcpu->arch.ipi_state.lock);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but is there a risk of lost
mailbox updates due to unprotected read-modify-write cycles in mail_send()
and send_ipi_data()?

In mail_send(), the code performs a read-modify-write cycle to update IPI
mailboxes using byte bit-masks:

    val = read_mailbox(vcpu, offset, 4);
    for (i = 0; i < 4; i++)
        if (data & (BIT(27 + i)))
            mask |= (0xff << (i * 8));
    val &= mask;
    ...
    val |= ((uint32_t)(data >> 32) & ~mask);
    write_mailbox(vcpu, offset, val, 4);

Since read_mailbox() and write_mailbox() acquire and release ipi_state.lock
internally, the lock is not held across the entire modification cycle. If
multiple vCPUs send IPIs with masks to the same target concurrently, could
their operations interleave and overwrite each other's updates?

[Severity: High]
This isn't a bug introduced by this patch, but can a guest deliberately
trigger a WARN_ON_ONCE by providing an unaligned IOCSR address?

Looking at loongarch_ipi_readl() and loongarch_ipi_writel(), they
unconditionally assert WARN_ON_ONCE for unaligned offsets:

    WARN_ON_ONCE(offset & (len - 1));

A guest can trigger this by executing an unaligned IOCSR read/write
instruction within the IPI address range, or by writing an unaligned offset
payload to IOCSR_ANY_SEND which reaches kvm_io_bus_read() or
kvm_io_bus_write() in send_ipi_data().

Since KVM io bus operations do not validate alignment before routing to
these functions, an unprivileged guest could intentionally panic the host
kernel on systems configured with panic_on_warn.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709082109.1361767-1-maobibo@loongson.cn?part=1

[PATCH v4 2/4] LoongArch: KVM: Replace kvm_err() with kvm_pr_unimpl()

[Bibo Mao]

Since guest kernel and ioctl parameter from user mode is untrusted,
there may be noise kernel log output in host hypervisor with abnormal
state. Here replace kvm_err() with kvm_pr_unimpl() to reduce this kind
of noise kernel log, and there is no function change.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 arch/loongarch/kvm/exit.c         | 16 ++++++++--------
 arch/loongarch/kvm/intc/dmsintc.c |  8 ++++----
 arch/loongarch/kvm/intc/eiointc.c | 18 ++++++++---------
 arch/loongarch/kvm/intc/ipi.c     | 32 +++++++++++++++----------------
 arch/loongarch/kvm/intc/pch_pic.c | 12 ++++++------
 arch/loongarch/kvm/vcpu.c         |  2 +-
 6 files changed, 44 insertions(+), 44 deletions(-)

diff --git a/arch/loongarch/kvm/exit.c b/arch/loongarch/kvm/exit.c
index 8572b63478bb..590580d0303e 100644
--- a/arch/loongarch/kvm/exit.c
+++ b/arch/loongarch/kvm/exit.c
@@ -252,7 +252,7 @@ int kvm_complete_iocsr_read(struct kvm_vcpu *vcpu, struct kvm_run *run)
 		*gpr = *(s64 *)run->iocsr_io.data;
 		break;
 	default:
-		kvm_err("Bad IOCSR length: %d, addr is 0x%lx\n",
+		kvm_pr_unimpl("Bad IOCSR length: %d, addr is 0x%lx\n",
 				run->iocsr_io.len, vcpu->arch.badv);
 		er = EMULATE_FAIL;
 		break;
@@ -326,7 +326,7 @@ static int kvm_trap_handle_gspr(struct kvm_vcpu *vcpu)
 
 	/* Rollback PC only if emulation was unsuccessful */
 	if (er == EMULATE_FAIL) {
-		kvm_err("[%#lx]%s: unsupported gspr instruction 0x%08x\n",
+		kvm_pr_unimpl("[%#lx]%s: unsupported gspr instruction 0x%08x\n",
 			curr_pc, __func__, inst.word);
 
 		kvm_arch_vcpu_dump_regs(vcpu);
@@ -491,7 +491,7 @@ int kvm_emu_mmio_read(struct kvm_vcpu *vcpu, larch_inst inst)
 		return EMULATE_DO_MMIO;
 	}
 
-	kvm_err("Read not supported Inst=0x%08x @%lx BadVaddr:%#lx\n",
+	kvm_pr_unimpl("Read not supported Inst=0x%08x @%lx BadVaddr:%#lx\n",
 			inst.word, vcpu->arch.pc, vcpu->arch.badv);
 	kvm_arch_vcpu_dump_regs(vcpu);
 	vcpu->mmio_needed = 0;
@@ -529,7 +529,7 @@ int kvm_complete_mmio_read(struct kvm_vcpu *vcpu, struct kvm_run *run)
 		*gpr = *(s64 *)run->mmio.data;
 		break;
 	default:
-		kvm_err("Bad MMIO length: %d, addr is 0x%lx\n",
+		kvm_pr_unimpl("Bad MMIO length: %d, addr is 0x%lx\n",
 				run->mmio.len, vcpu->arch.badv);
 		er = EMULATE_FAIL;
 		break;
@@ -656,7 +656,7 @@ int kvm_emu_mmio_write(struct kvm_vcpu *vcpu, larch_inst inst)
 	}
 
 	vcpu->arch.pc = curr_pc;
-	kvm_err("Write not supported Inst=0x%08x @%lx BadVaddr:%#lx\n",
+	kvm_pr_unimpl("Write not supported Inst=0x%08x @%lx BadVaddr:%#lx\n",
 			inst.word, vcpu->arch.pc, vcpu->arch.badv);
 	kvm_arch_vcpu_dump_regs(vcpu);
 	/* Rollback PC if emulation was unsuccessful */
@@ -748,8 +748,8 @@ static int kvm_handle_fpu_disabled(struct kvm_vcpu *vcpu, int ecode)
 	 * treated as a reserved instruction!
 	 * If FPU already in use, we shouldn't get this at all.
 	 */
-	if (WARN_ON(vcpu->arch.aux_inuse & KVM_LARCH_FPU)) {
-		kvm_err("%s internal error\n", __func__);
+	if (vcpu->arch.aux_inuse & KVM_LARCH_FPU) {
+		kvm_pr_unimpl("%s internal error\n", __func__);
 		run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
 		return RESUME_HOST;
 	}
@@ -943,7 +943,7 @@ static int kvm_fault_ni(struct kvm_vcpu *vcpu, int ecode)
 	/* Fetch the instruction */
 	inst = vcpu->arch.badi;
 	badv = vcpu->arch.badv;
-	kvm_err("ECode: %d PC=%#lx Inst=0x%08x BadVaddr=%#lx ESTAT=%#lx\n",
+	kvm_pr_unimpl("ECode: %d PC=%#lx Inst=0x%08x BadVaddr=%#lx ESTAT=%#lx\n",
 			ecode, vcpu->arch.pc, inst, badv, read_gcsr_estat());
 	kvm_arch_vcpu_dump_regs(vcpu);
 	kvm_queue_exception(vcpu, EXCCODE_INE, 0);
diff --git a/arch/loongarch/kvm/intc/dmsintc.c b/arch/loongarch/kvm/intc/dmsintc.c
index de25735ce039..622d3127d898 100644
--- a/arch/loongarch/kvm/intc/dmsintc.c
+++ b/arch/loongarch/kvm/intc/dmsintc.c
@@ -114,7 +114,7 @@ static int kvm_dmsintc_ctrl_access(struct kvm_device *dev,
 		}
 		break;
 	default:
-		kvm_err("%s: unknown dmsintc register, addr = %d\n", __func__, addr);
+		kvm_pr_unimpl("%s: unknown dmsintc register, addr = %d\n", __func__, addr);
 		return -ENXIO;
 	}
 
@@ -128,7 +128,7 @@ static int kvm_dmsintc_set_attr(struct kvm_device *dev,
 	case KVM_DEV_LOONGARCH_DMSINTC_GRP_CTRL:
 		return kvm_dmsintc_ctrl_access(dev, attr, true);
 	default:
-		kvm_err("%s: unknown group (%d)\n", __func__, attr->group);
+		kvm_pr_unimpl("%s: unknown group (%d)\n", __func__, attr->group);
 		return -EINVAL;
 	}
 }
@@ -139,13 +139,13 @@ static int kvm_dmsintc_create(struct kvm_device *dev, u32 type)
 	struct loongarch_dmsintc *s;
 
 	if (!dev) {
-		kvm_err("%s: kvm_device ptr is invalid!\n", __func__);
+		kvm_pr_unimpl("%s: kvm_device ptr is invalid!\n", __func__);
 		return -EINVAL;
 	}
 
 	kvm = dev->kvm;
 	if (kvm->arch.dmsintc) {
-		kvm_err("%s: LoongArch DMSINTC has already been created!\n", __func__);
+		kvm_pr_unimpl("%s: LoongArch DMSINTC has already been created!\n", __func__);
 		return -EINVAL;
 	}
 
diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
index 2b14485d14a7..fd089d54da10 100644
--- a/arch/loongarch/kvm/intc/eiointc.c
+++ b/arch/loongarch/kvm/intc/eiointc.c
@@ -47,7 +47,7 @@ static void eiointc_update_irq(struct loongarch_eiointc *s, int irq, int level)
 	cpu = s->sw_coremap[irq];
 	vcpu = kvm_get_vcpu_by_id(s->kvm, cpu);
 	if (unlikely(vcpu == NULL)) {
-		kvm_err("%s: invalid target cpu: %d\n", __func__, cpu);
+		kvm_pr_unimpl("%s: invalid target cpu: %d\n", __func__, cpu);
 		return;
 	}
 
@@ -165,12 +165,12 @@ static int kvm_eiointc_read(struct kvm_vcpu *vcpu,
 	struct loongarch_eiointc *eiointc = vcpu->kvm->arch.eiointc;
 
 	if (!eiointc) {
-		kvm_err("%s: eiointc irqchip not valid!\n", __func__);
+		kvm_pr_unimpl("%s: eiointc irqchip not valid!\n", __func__);
 		return 0;
 	}
 
 	if (addr & (len - 1)) {
-		kvm_err("%s: eiointc not aligned addr %llx len %d\n", __func__, addr, len);
+		kvm_pr_unimpl("%s: eiointc not aligned addr %llx len %d\n", __func__, addr, len);
 		return 0;
 	}
 
@@ -297,12 +297,12 @@ static int kvm_eiointc_write(struct kvm_vcpu *vcpu,
 	struct loongarch_eiointc *eiointc = vcpu->kvm->arch.eiointc;
 
 	if (!eiointc) {
-		kvm_err("%s: eiointc irqchip not valid!\n", __func__);
+		kvm_pr_unimpl("%s: eiointc irqchip not valid!\n", __func__);
 		return 0;
 	}
 
 	if (addr & (len - 1)) {
-		kvm_err("%s: eiointc not aligned addr %llx len %d\n", __func__, addr, len);
+		kvm_pr_unimpl("%s: eiointc not aligned addr %llx len %d\n", __func__, addr, len);
 		return 0;
 	}
 
@@ -345,7 +345,7 @@ static int kvm_eiointc_virt_read(struct kvm_vcpu *vcpu,
 	struct loongarch_eiointc *eiointc = vcpu->kvm->arch.eiointc;
 
 	if (!eiointc) {
-		kvm_err("%s: eiointc irqchip not valid!\n", __func__);
+		kvm_pr_unimpl("%s: eiointc irqchip not valid!\n", __func__);
 		return 0;
 	}
 
@@ -375,7 +375,7 @@ static int kvm_eiointc_virt_write(struct kvm_vcpu *vcpu,
 	struct loongarch_eiointc *eiointc = vcpu->kvm->arch.eiointc;
 
 	if (!eiointc) {
-		kvm_err("%s: eiointc irqchip not valid!\n", __func__);
+		kvm_pr_unimpl("%s: eiointc irqchip not valid!\n", __func__);
 		return 0;
 	}
 
@@ -502,7 +502,7 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
 		p = (void *)s->coremap + offset * 4;
 		break;
 	default:
-		kvm_err("%s: unknown eiointc register, addr = %d\n", __func__, addr);
+		kvm_pr_unimpl("%s: unknown eiointc register, addr = %d\n", __func__, addr);
 		return -EINVAL;
 	}
 
@@ -546,7 +546,7 @@ static int kvm_eiointc_sw_status_access(struct kvm_device *dev,
 		p = &s->status;
 		break;
 	default:
-		kvm_err("%s: unknown eiointc register, addr = %d\n", __func__, addr);
+		kvm_pr_unimpl("%s: unknown eiointc register, addr = %d\n", __func__, addr);
 		return -EINVAL;
 	}
 	spin_lock_irqsave(&s->lock, flags);
diff --git a/arch/loongarch/kvm/intc/ipi.c b/arch/loongarch/kvm/intc/ipi.c
index d70bf415c7fc..bccdfd963e5b 100644
--- a/arch/loongarch/kvm/intc/ipi.c
+++ b/arch/loongarch/kvm/intc/ipi.c
@@ -30,7 +30,7 @@ static void ipi_send(struct kvm *kvm, uint64_t data)
 	cpu = ((data & 0xffffffff) >> 16) & 0x3ff;
 	vcpu = kvm_get_vcpu_by_cpuid(kvm, cpu);
 	if (unlikely(vcpu == NULL)) {
-		kvm_err("%s: invalid target cpu: %d\n", __func__, cpu);
+		kvm_pr_unimpl("%s: invalid target cpu: %d\n", __func__, cpu);
 		return;
 	}
 
@@ -105,7 +105,7 @@ static int mail_send(struct kvm *kvm, uint64_t data)
 	cpu = ((data & 0xffffffff) >> 16) & 0x3ff;
 	vcpu = kvm_get_vcpu_by_cpuid(kvm, cpu);
 	if (unlikely(vcpu == NULL)) {
-		kvm_err("%s: invalid target cpu: %d\n", __func__, cpu);
+		kvm_pr_unimpl("%s: invalid target cpu: %d\n", __func__, cpu);
 		return 0;
 	}
 	mailbox = ((data & 0xffffffff) >> 2) & 0x7;
@@ -139,7 +139,7 @@ static int send_ipi_data(struct kvm_vcpu *vcpu, gpa_t addr, uint64_t data)
 		ret = kvm_io_bus_read(vcpu, KVM_IOCSR_BUS, addr, 4, &val);
 		srcu_read_unlock(&vcpu->kvm->srcu, idx);
 		if (unlikely(ret)) {
-			kvm_err("%s: : read data from addr %llx failed\n", __func__, addr);
+			kvm_pr_unimpl("%s: : read data from addr %llx failed\n", __func__, addr);
 			return 0;
 		}
 		/* Construct the mask by scanning the bit 27-30 */
@@ -155,7 +155,7 @@ static int send_ipi_data(struct kvm_vcpu *vcpu, gpa_t addr, uint64_t data)
 	ret = kvm_io_bus_write(vcpu, KVM_IOCSR_BUS, addr, 4, &val);
 	srcu_read_unlock(&vcpu->kvm->srcu, idx);
 	if (unlikely(ret))
-		kvm_err("%s: : write data to addr %llx failed\n", __func__, addr);
+		kvm_pr_unimpl("%s: : write data to addr %llx failed\n", __func__, addr);
 
 	return 0;
 }
@@ -168,7 +168,7 @@ static int any_send(struct kvm *kvm, uint64_t data)
 	cpu = ((data & 0xffffffff) >> 16) & 0x3ff;
 	vcpu = kvm_get_vcpu_by_cpuid(kvm, cpu);
 	if (unlikely(vcpu == NULL)) {
-		kvm_err("%s: invalid target cpu: %d\n", __func__, cpu);
+		kvm_pr_unimpl("%s: invalid target cpu: %d\n", __func__, cpu);
 		return 0;
 	}
 	offset = data & 0xffff;
@@ -200,14 +200,14 @@ static int loongarch_ipi_readl(struct kvm_vcpu *vcpu, gpa_t addr, int len, void
 		break;
 	case IOCSR_IPI_BUF_20 ... IOCSR_IPI_BUF_38 + 7:
 		if (offset + len > IOCSR_IPI_BUF_38 + 8) {
-			kvm_err("%s: invalid offset or len: offset = %d, len = %d\n",
+			kvm_pr_unimpl("%s: invalid offset or len: offset = %d, len = %d\n",
 				__func__, offset, len);
 			break;
 		}
 		res = read_mailbox(vcpu, offset, len);
 		break;
 	default:
-		kvm_err("%s: unknown addr: %llx\n", __func__, addr);
+		kvm_pr_unimpl("%s: unknown addr: %llx\n", __func__, addr);
 		break;
 	}
 	*(uint64_t *)val = res;
@@ -242,7 +242,7 @@ static int loongarch_ipi_writel(struct kvm_vcpu *vcpu, gpa_t addr, int len, cons
 		break;
 	case IOCSR_IPI_BUF_20 ... IOCSR_IPI_BUF_38 + 7:
 		if (offset + len > IOCSR_IPI_BUF_38 + 8) {
-			kvm_err("%s: invalid offset or len: offset = %d, len = %d\n",
+			kvm_pr_unimpl("%s: invalid offset or len: offset = %d, len = %d\n",
 				__func__, offset, len);
 			break;
 		}
@@ -258,7 +258,7 @@ static int loongarch_ipi_writel(struct kvm_vcpu *vcpu, gpa_t addr, int len, cons
 		any_send(vcpu->kvm, data);
 		break;
 	default:
-		kvm_err("%s: unknown addr: %llx\n", __func__, addr);
+		kvm_pr_unimpl("%s: unknown addr: %llx\n", __func__, addr);
 		break;
 	}
 
@@ -301,7 +301,7 @@ static int kvm_ipi_regs_access(struct kvm_device *dev,
 
 	vcpu = kvm_get_vcpu_by_id(dev->kvm, cpu);
 	if (unlikely(vcpu == NULL)) {
-		kvm_err("%s: invalid target cpu: %d\n", __func__, cpu);
+		kvm_pr_unimpl("%s: invalid target cpu: %d\n", __func__, cpu);
 		return -EINVAL;
 	}
 
@@ -335,7 +335,7 @@ static int kvm_ipi_regs_access(struct kvm_device *dev,
 		len = 8;
 		break;
 	default:
-		kvm_err("%s: unknown ipi register, addr = %d\n", __func__, addr);
+		kvm_pr_unimpl("%s: unknown ipi register, addr = %d\n", __func__, addr);
 		return -EINVAL;
 	}
 
@@ -369,7 +369,7 @@ static int kvm_ipi_get_attr(struct kvm_device *dev,
 	case KVM_DEV_LOONGARCH_IPI_GRP_REGS:
 		return kvm_ipi_regs_access(dev, attr, false);
 	default:
-		kvm_err("%s: unknown group (%d)\n", __func__, attr->group);
+		kvm_pr_unimpl("%s: unknown group (%d)\n", __func__, attr->group);
 		return -EINVAL;
 	}
 }
@@ -381,7 +381,7 @@ static int kvm_ipi_set_attr(struct kvm_device *dev,
 	case KVM_DEV_LOONGARCH_IPI_GRP_REGS:
 		return kvm_ipi_regs_access(dev, attr, true);
 	default:
-		kvm_err("%s: unknown group (%d)\n", __func__, attr->group);
+		kvm_pr_unimpl("%s: unknown group (%d)\n", __func__, attr->group);
 		return -EINVAL;
 	}
 }
@@ -394,13 +394,13 @@ static int kvm_ipi_create(struct kvm_device *dev, u32 type)
 	struct loongarch_ipi *s;
 
 	if (!dev) {
-		kvm_err("%s: kvm_device ptr is invalid!\n", __func__);
+		kvm_pr_unimpl("%s: kvm_device ptr is invalid!\n", __func__);
 		return -EINVAL;
 	}
 
 	kvm = dev->kvm;
 	if (kvm->arch.ipi) {
-		kvm_err("%s: LoongArch IPI has already been created!\n", __func__);
+		kvm_pr_unimpl("%s: LoongArch IPI has already been created!\n", __func__);
 		return -EINVAL;
 	}
 
@@ -420,7 +420,7 @@ static int kvm_ipi_create(struct kvm_device *dev, u32 type)
 	ret = kvm_io_bus_register_dev(kvm, KVM_IOCSR_BUS, IOCSR_IPI_BASE, IOCSR_IPI_SIZE, device);
 	mutex_unlock(&kvm->slots_lock);
 	if (ret < 0) {
-		kvm_err("%s: Initialize IOCSR dev failed, ret = %d\n", __func__, ret);
+		kvm_pr_unimpl("%s: Initialize IOCSR dev failed, ret = %d\n", __func__, ret);
 		goto err;
 	}
 
diff --git a/arch/loongarch/kvm/intc/pch_pic.c b/arch/loongarch/kvm/intc/pch_pic.c
index 175a630aceb4..e7b77705c516 100644
--- a/arch/loongarch/kvm/intc/pch_pic.c
+++ b/arch/loongarch/kvm/intc/pch_pic.c
@@ -151,12 +151,12 @@ static int kvm_pch_pic_read(struct kvm_vcpu *vcpu,
 	struct loongarch_pch_pic *s = vcpu->kvm->arch.pch_pic;
 
 	if (!s) {
-		kvm_err("%s: pch pic irqchip not valid!\n", __func__);
+		kvm_pr_unimpl("%s: pch pic irqchip not valid!\n", __func__);
 		return ret;
 	}
 
 	if (addr & (len - 1)) {
-		kvm_err("%s: pch pic not aligned addr %llx len %d\n", __func__, addr, len);
+		kvm_pr_unimpl("%s: pch pic not aligned addr %llx len %d\n", __func__, addr, len);
 		return ret;
 	}
 
@@ -250,12 +250,12 @@ static int kvm_pch_pic_write(struct kvm_vcpu *vcpu,
 	struct loongarch_pch_pic *s = vcpu->kvm->arch.pch_pic;
 
 	if (!s) {
-		kvm_err("%s: pch pic irqchip not valid!\n", __func__);
+		kvm_pr_unimpl("%s: pch pic irqchip not valid!\n", __func__);
 		return ret;
 	}
 
 	if (addr & (len - 1)) {
-		kvm_err("%s: pch pic not aligned addr %llx len %d\n", __func__, addr, len);
+		kvm_pr_unimpl("%s: pch pic not aligned addr %llx len %d\n", __func__, addr, len);
 		return ret;
 	}
 
@@ -390,13 +390,13 @@ static int kvm_pch_pic_set_attr(struct kvm_device *dev,
 				return -EFAULT;
 
 			if (!dev->kvm->arch.pch_pic) {
-				kvm_err("%s: please create pch_pic irqchip first!\n", __func__);
+				kvm_pr_unimpl("%s: pch_pic irqchip not created\n", __func__);
 				return -ENODEV;
 			}
 
 			return kvm_pch_pic_init(dev, addr);
 		default:
-			kvm_err("%s: unknown group (%d) attr (%lld)\n", __func__, attr->group,
+			kvm_pr_unimpl("%s: unknown group (%d) attr (%lld)\n", __func__, attr->group,
 					attr->attr);
 			return -EINVAL;
 		}
diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
index 20c207d80e31..1a0cf6c81df6 100644
--- a/arch/loongarch/kvm/vcpu.c
+++ b/arch/loongarch/kvm/vcpu.c
@@ -1473,7 +1473,7 @@ int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu, struct kvm_interrupt *irq)
 	else if (intr < 0)
 		kvm_dequeue_irq(vcpu, -intr);
 	else {
-		kvm_err("%s: invalid interrupt ioctl %d\n", __func__, irq->irq);
+		kvm_pr_unimpl("%s: invalid interrupt ioctl %d\n", __func__, irq->irq);
 		return -EINVAL;
 	}
 
-- 
2.39.3

[PATCH v4 3/4] LoongArch: KVM: Return directly when IPI address is not aligned

[Bibo Mao]

When IPI address is not aligned with its access size, it should return
directly in read/write operations, rather than only print WARN_ON_ONCE().
The method is the same with in kernel extioi and pch_pic operations.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 arch/loongarch/kvm/intc/ipi.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/arch/loongarch/kvm/intc/ipi.c b/arch/loongarch/kvm/intc/ipi.c
index bccdfd963e5b..8ec40e09c5ab 100644
--- a/arch/loongarch/kvm/intc/ipi.c
+++ b/arch/loongarch/kvm/intc/ipi.c
@@ -181,9 +181,12 @@ static int loongarch_ipi_readl(struct kvm_vcpu *vcpu, gpa_t addr, int len, void
 	uint32_t offset;
 	uint64_t res = 0;
 
-	offset = (uint32_t)(addr & 0x1ff);
-	WARN_ON_ONCE(offset & (len - 1));
+	if (addr & (len - 1)) {
+		kvm_pr_unimpl("%s: ipi not aligned addr %llx len %d\n", __func__, addr, len);
+		return 0;
+	}
 
+	offset = addr - IOCSR_IPI_BASE;
 	switch (offset) {
 	case IOCSR_IPI_STATUS:
 		spin_lock(&vcpu->arch.ipi_state.lock);
@@ -199,11 +202,6 @@ static int loongarch_ipi_readl(struct kvm_vcpu *vcpu, gpa_t addr, int len, void
 	case IOCSR_IPI_CLEAR:
 		break;
 	case IOCSR_IPI_BUF_20 ... IOCSR_IPI_BUF_38 + 7:
-		if (offset + len > IOCSR_IPI_BUF_38 + 8) {
-			kvm_pr_unimpl("%s: invalid offset or len: offset = %d, len = %d\n",
-				__func__, offset, len);
-			break;
-		}
 		res = read_mailbox(vcpu, offset, len);
 		break;
 	default:
@@ -222,9 +220,12 @@ static int loongarch_ipi_writel(struct kvm_vcpu *vcpu, gpa_t addr, int len, cons
 
 	data = *(uint64_t *)val;
 
-	offset = (uint32_t)(addr & 0x1ff);
-	WARN_ON_ONCE(offset & (len - 1));
+	if (addr & (len - 1)) {
+		kvm_pr_unimpl("%s: ipi not aligned addr %llx len %d\n", __func__, addr, len);
+		return 0;
+	}
 
+	offset = addr - IOCSR_IPI_BASE;
 	switch (offset) {
 	case IOCSR_IPI_STATUS:
 		break;
@@ -241,11 +242,6 @@ static int loongarch_ipi_writel(struct kvm_vcpu *vcpu, gpa_t addr, int len, cons
 		ipi_clear(vcpu, data);
 		break;
 	case IOCSR_IPI_BUF_20 ... IOCSR_IPI_BUF_38 + 7:
-		if (offset + len > IOCSR_IPI_BUF_38 + 8) {
-			kvm_pr_unimpl("%s: invalid offset or len: offset = %d, len = %d\n",
-				__func__, offset, len);
-			break;
-		}
 		write_mailbox(vcpu, offset, data, len);
 		break;
 	case IOCSR_IPI_SEND:
-- 
2.39.3

Re: [PATCH v4 3/4] LoongArch: KVM: Return directly when IPI address is not aligned

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The early return path for unaligned accesses in `loongarch_ipi_readl` fails to populate the `val` output parameter, leaking stale data to the guest.

Pre-existing issues:
- [Critical] A malicious guest can trigger a host kernel panic by performing an unaligned MMIO access to the MIPS KVM IPI region.
--

--- Patch [3]: [PATCH v4 3/4] LoongArch: KVM: Return directly when IPI address is not aligned ---
commit 8225fc6b95e2b4712a3ffe27c14012171a25b47b
Author: Bibo Mao <maobibo@loongson.cn>

LoongArch: KVM: Return directly when IPI address is not aligned

This patch modifies KVM on LoongArch to return directly from IPI read and
write operations when the address is unaligned, rather than just printing a
warning.

> diff --git a/arch/loongarch/kvm/intc/ipi.c b/arch/loongarch/kvm/intc/ipi.c
> index bccdfd963e5b5..8ec40e09c5abb 100644
> --- a/arch/loongarch/kvm/intc/ipi.c
> +++ b/arch/loongarch/kvm/intc/ipi.c
> @@ -181,9 +181,12 @@ static int loongarch_ipi_readl(struct kvm_vcpu *vcpu, gpa_t addr, int len, void
>  	uint32_t offset;
>  	uint64_t res = 0;
>  
> -	offset = (uint32_t)(addr & 0x1ff);
> -	WARN_ON_ONCE(offset & (len - 1));
> +	if (addr & (len - 1)) {
> +		kvm_pr_unimpl("%s: ipi not aligned addr %llx len %d\n", __func__, addr, len);
> +		return 0;
> +	}

[Severity: High]
Does this early return path in loongarch_ipi_readl() need to populate the val
output parameter before returning success (0)?

If a guest performs an unaligned read, kvm_io_bus_read() will see a successful
return, but val remains unmodified. Could this lead to stale data from
previous VM exits being copied into the guest's registers?

[ ... ]

[Severity: Critical]
This is a pre-existing issue and was not introduced by this patch, but while
reviewing unaligned IPI access handling, a guest-triggerable BUG_ON() was
noticed in the MIPS loongson_vipi_read() and loongson_vipi_write() paths:

arch/mips/kvm/loongson_ipi.c:loongson_vipi_read() {
    ...
    BUG_ON(offset & (len - 1));
    ...
}

Could a malicious guest trigger a host kernel panic by performing an unaligned
MMIO read or write targeting the loongson_ipi region?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709082109.1361767-1-maobibo@loongson.cn?part=3

[PATCH v4 4/4] LoongArch: KVM: Use internal API to deliver interrupt in kernel mode

[Bibo Mao]

API kvm_vcpu_ioctl_interrupt() is mainly to used to deliver interrupt
from user mode, and internal APIs kvm_queue_irq() and kvm_dequeue_irq()
are used in kernel mode.

Also move IPI inject and ack within lock protection to avoid contention
in ipi_set() and ipi_clear().

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 arch/loongarch/kvm/intc/dmsintc.c |  7 ++++---
 arch/loongarch/kvm/intc/eiointc.c | 14 +++++++++-----
 arch/loongarch/kvm/intc/ipi.c     | 14 ++++++--------
 arch/loongarch/kvm/vcpu.c         |  3 +++
 4 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/arch/loongarch/kvm/intc/dmsintc.c b/arch/loongarch/kvm/intc/dmsintc.c
index 622d3127d898..8e538fc0eeca 100644
--- a/arch/loongarch/kvm/intc/dmsintc.c
+++ b/arch/loongarch/kvm/intc/dmsintc.c
@@ -47,7 +47,6 @@ void dmsintc_inject_irq(struct kvm_vcpu *vcpu)
 int dmsintc_deliver_msi_to_vcpu(struct kvm *kvm,
 				struct kvm_vcpu *vcpu, u32 vector, int level)
 {
-	struct kvm_interrupt vcpu_irq;
 	struct dmsintc_state *ds = &vcpu->arch.dmsintc_state;
 
 	if (!level)
@@ -57,9 +56,11 @@ int dmsintc_deliver_msi_to_vcpu(struct kvm *kvm,
 	if (!ds)
 		return -ENODEV;
 
-	vcpu_irq.irq = INT_AVEC;
+	if (!kvm_guest_has_msgint(&vcpu->arch))
+		return -EINVAL;
+
 	set_bit(vector, (unsigned long *)&ds->vector_map);
-	kvm_vcpu_ioctl_interrupt(vcpu, &vcpu_irq);
+	kvm_queue_irq(vcpu, INT_AVEC);
 	kvm_vcpu_kick(vcpu);
 
 	return 0;
diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
index fd089d54da10..e04aadbd68db 100644
--- a/arch/loongarch/kvm/intc/eiointc.c
+++ b/arch/loongarch/kvm/intc/eiointc.c
@@ -17,7 +17,8 @@ static void eiointc_set_sw_coreisr(struct loongarch_eiointc *s)
 		if (!(s->status & BIT(EIOINTC_ENABLE_INT_ENCODE))) {
 			ipnum = count_trailing_zeros(ipnum);
 			ipnum = ipnum < 4 ? ipnum : 0;
-		}
+		} else if (ipnum >= LOONGSON_IP_NUM)
+			ipnum = 0;
 
 		cpuid = ((u8 *)s->coremap)[irq];
 		vcpu = kvm_get_vcpu_by_cpuid(s->kvm, cpuid);
@@ -36,13 +37,13 @@ static void eiointc_update_irq(struct loongarch_eiointc *s, int irq, int level)
 {
 	int ipnum, cpu, found;
 	struct kvm_vcpu *vcpu;
-	struct kvm_interrupt vcpu_irq;
 
 	ipnum = (s->ipmap >> (irq / 32 * 8)) & 0xff;
 	if (!(s->status & BIT(EIOINTC_ENABLE_INT_ENCODE))) {
 		ipnum = count_trailing_zeros(ipnum);
 		ipnum = ipnum < 4 ? ipnum : 0;
-	}
+	} else if (ipnum >= LOONGSON_IP_NUM)
+		ipnum = 0;
 
 	cpu = s->sw_coremap[irq];
 	vcpu = kvm_get_vcpu_by_id(s->kvm, cpu);
@@ -67,8 +68,11 @@ static void eiointc_update_irq(struct loongarch_eiointc *s, int irq, int level)
 	if (found < EIOINTC_IRQS)
 		return; /* other irq is handling, needn't update parent irq */
 
-	vcpu_irq.irq = level ? (INT_HWI0 + ipnum) : -(INT_HWI0 + ipnum);
-	kvm_vcpu_ioctl_interrupt(vcpu, &vcpu_irq);
+	if (level)
+		kvm_queue_irq(vcpu, INT_HWI0 + ipnum);
+	else
+		kvm_dequeue_irq(vcpu, INT_HWI0 + ipnum);
+	kvm_vcpu_kick(vcpu);
 }
 
 static inline void eiointc_update_sw_coremap(struct loongarch_eiointc *s,
diff --git a/arch/loongarch/kvm/intc/ipi.c b/arch/loongarch/kvm/intc/ipi.c
index 8ec40e09c5ab..603b04704d12 100644
--- a/arch/loongarch/kvm/intc/ipi.c
+++ b/arch/loongarch/kvm/intc/ipi.c
@@ -10,16 +10,15 @@
 static void ipi_set(struct kvm_vcpu *vcpu, uint32_t data)
 {
 	uint32_t status;
-	struct kvm_interrupt irq;
 
 	spin_lock(&vcpu->arch.ipi_state.lock);
 	status = vcpu->arch.ipi_state.status;
 	vcpu->arch.ipi_state.status |= data;
-	spin_unlock(&vcpu->arch.ipi_state.lock);
 	if ((status == 0) && data) {
-		irq.irq = LARCH_INT_IPI;
-		kvm_vcpu_ioctl_interrupt(vcpu, &irq);
+		kvm_queue_irq(vcpu, LARCH_INT_IPI);
+		kvm_vcpu_kick(vcpu);
 	}
+	spin_unlock(&vcpu->arch.ipi_state.lock);
 }
 
 static void ipi_send(struct kvm *kvm, uint64_t data)
@@ -40,16 +39,15 @@ static void ipi_send(struct kvm *kvm, uint64_t data)
 static void ipi_clear(struct kvm_vcpu *vcpu, uint64_t data)
 {
 	uint32_t status;
-	struct kvm_interrupt irq;
 
 	spin_lock(&vcpu->arch.ipi_state.lock);
 	vcpu->arch.ipi_state.status &= ~data;
 	status = vcpu->arch.ipi_state.status;
-	spin_unlock(&vcpu->arch.ipi_state.lock);
 	if (status == 0) {
-		irq.irq = -LARCH_INT_IPI;
-		kvm_vcpu_ioctl_interrupt(vcpu, &irq);
+		kvm_dequeue_irq(vcpu, LARCH_INT_IPI);
+		kvm_vcpu_kick(vcpu);
 	}
+	spin_unlock(&vcpu->arch.ipi_state.lock);
 }
 
 static uint64_t read_mailbox(struct kvm_vcpu *vcpu, int offset, int len)
diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
index 1a0cf6c81df6..f81dadc62c98 100644
--- a/arch/loongarch/kvm/vcpu.c
+++ b/arch/loongarch/kvm/vcpu.c
@@ -1465,6 +1465,9 @@ int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu, struct kvm_interrupt *irq)
 	if (vector >= EXCCODE_INT_NUM)
 		return -EINVAL;
 
+	if (kvm_arch_irqchip_in_kernel(vcpu->kvm))
+		return -EINVAL;
+
 	if (!kvm_guest_has_msgint(&vcpu->arch) && (vector == INT_AVEC))
 		return -EINVAL;
 
-- 
2.39.3

Re: [PATCH v4 4/4] LoongArch: KVM: Use internal API to deliver interrupt in kernel mode

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Undocumented Security Fix / Out-of-Bounds Access
- [Medium] Undocumented API Contract Change for KVM_INTERRUPT
--

--- Patch [4]: [PATCH v4 4/4] LoongArch: KVM: Use internal API to deliver interrupt in kernel mode ---
commit b41f003908862410927912c7ede144c461f9fa1e
Author: Bibo Mao <maobibo@loongson.cn>

LoongArch: KVM: Use internal API to deliver interrupt in kernel mode

This commit updates the KVM LoongArch code to use internal APIs
kvm_queue_irq() and kvm_dequeue_irq() for kernel-mode interrupt delivery,
reserving kvm_vcpu_ioctl_interrupt() for user-mode. It also moves IPI
injection and ack operations inside lock protection to avoid contention.

> diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
> index fd089d54da103..e04aadbd68db9 100644
> --- a/arch/loongarch/kvm/intc/eiointc.c
> +++ b/arch/loongarch/kvm/intc/eiointc.c
> @@ -17,7 +17,8 @@ static void eiointc_set_sw_coreisr(struct loongarch_eiointc *s)
>  		if (!(s->status & BIT(EIOINTC_ENABLE_INT_ENCODE))) {
>  			ipnum = count_trailing_zeros(ipnum);
>  			ipnum = ipnum < 4 ? ipnum : 0;
> -		}
> +		} else if (ipnum >= LOONGSON_IP_NUM)
> +			ipnum = 0;

[Severity: Medium]
This isn't a bug introduced by this patch, but the commit message doesn't
mention adding these bounds checks. 

Since ipnum is used as an array index into the sw_coreisr array shortly after
this in eiointc_set_sw_coreisr(), does this fix a guest-triggerable
out-of-bounds access?

If so, should this security fix be documented in the commit message or perhaps
split into a separate patch?

[ ... ]
> @@ -36,13 +37,13 @@ static void eiointc_update_irq(struct loongarch_eiointc *s, int irq, int level)
>  {
>  	int ipnum, cpu, found;
>  	struct kvm_vcpu *vcpu;
> -	struct kvm_interrupt vcpu_irq;
>  
>  	ipnum = (s->ipmap >> (irq / 32 * 8)) & 0xff;
>  	if (!(s->status & BIT(EIOINTC_ENABLE_INT_ENCODE))) {
>  		ipnum = count_trailing_zeros(ipnum);
>  		ipnum = ipnum < 4 ? ipnum : 0;
> -	}
> +	} else if (ipnum >= LOONGSON_IP_NUM)
> +		ipnum = 0;

[Severity: Medium]
Similarly, this bounds check in eiointc_update_irq() appears to prevent
out-of-bounds accesses but is undocumented in the commit message.

[ ... ]
> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> index 1a0cf6c81df6a..f81dadc62c986 100644
> --- a/arch/loongarch/kvm/vcpu.c
> +++ b/arch/loongarch/kvm/vcpu.c
> @@ -1465,6 +1465,9 @@ int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu, struct kvm_interrupt *irq)
>  	if (vector >= EXCCODE_INT_NUM)
>  		return -EINVAL;
>  
> +	if (kvm_arch_irqchip_in_kernel(vcpu->kvm))
> +		return -EINVAL;
> +

[Severity: Medium]
Does this change break backwards compatibility for any userspace applications
that might be calling the KVM_INTERRUPT ioctl while the in-kernel irqchip
is enabled?

While it makes architectural sense to prevent userspace from concurrently
injecting raw interrupts in this state, the commit message does not explicitly
mention this change to the user-facing API contract. Could the commit message 
be updated to clearly document this new restriction?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709082109.1361767-1-maobibo@loongson.cn?part=4