On 2025/6/20 下午10:43, Huacai Chen wrote:
On Fri, Jun 20, 2025 at 9:43 AM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:
On 2025/6/19 下午4:46, Huacai Chen wrote:
Hi, Bibo,
On Wed, Jun 11, 2025 at 9:47 AM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:
The maximum supported cpu number is EIOINTC_ROUTE_MAX_VCPUS about
irqchip eiointc, here add validation about cpu number to avoid array
pointer overflow.
Cc: stable@xxxxxxxxxxxxxxx
Fixes: 1ad7efa552fd ("LoongArch: KVM: Add EIOINTC user mode read and write functions")
Signed-off-by: Bibo Mao <maobibo@xxxxxxxxxxx>
---
arch/loongarch/kvm/intc/eiointc.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
index b48511f903b5..ed80bf290755 100644
--- a/arch/loongarch/kvm/intc/eiointc.c
+++ b/arch/loongarch/kvm/intc/eiointc.c
@@ -798,7 +798,7 @@ static int kvm_eiointc_ctrl_access(struct kvm_device *dev,
int ret = 0;
unsigned long flags;
unsigned long type = (unsigned long)attr->attr;
- u32 i, start_irq;
+ u32 i, start_irq, val;
void __user *data;
struct loongarch_eiointc *s = dev->kvm->arch.eiointc;
@@ -806,7 +806,12 @@ static int kvm_eiointc_ctrl_access(struct kvm_device *dev,
spin_lock_irqsave(&s->lock, flags);
switch (type) {
case KVM_DEV_LOONGARCH_EXTIOI_CTRL_INIT_NUM_CPU:
- if (copy_from_user(&s->num_cpu, data, 4))
+ if (copy_from_user(&val, data, 4) == 0) {
+ if (val < EIOINTC_ROUTE_MAX_VCPUS)
+ s->num_cpu = val;
+ else
+ ret = -EINVAL;
Maybe it is better to set s->num_cpu to EIOINTC_ROUTE_MAX_VCPUS (or
other value) rather than keep it uninitialized. Because in other
places we need to check s->num_cpu and an uninitialized value may
cause undefined behavior.
There is error return value -EINVAL, VMM should stop running and exit
immediately if there is error return value with the ioctl command.
num_cpu is not uninitialized and it is zero by default. If VMM does not
care about the return value, VMM will fail to get coreisr information in
future.
If you are sure you can keep it as is. Then please resend patch
1,2,3,4,5,9 as a series because they are all bug fixes that should be
merged as soon as possible. And in my own opinion, "INTC" can be
dropped in the title.
Ok, will do in this way.
Regards
Bibo Mao
Huacai
Regards
Bibo Mao
Huacai
+ } else
ret = -EFAULT;
break;
case KVM_DEV_LOONGARCH_EXTIOI_CTRL_INIT_FEATURE:
@@ -835,7 +840,7 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
struct kvm_device_attr *attr,
bool is_write)
{
- int addr, cpuid, offset, ret = 0;
+ int addr, cpu, offset, ret = 0;
unsigned long flags;
void *p = NULL;
void __user *data;
@@ -843,7 +848,7 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
s = dev->kvm->arch.eiointc;
addr = attr->attr;
- cpuid = addr >> 16;
+ cpu = addr >> 16;
addr &= 0xffff;
data = (void __user *)attr->addr;
switch (addr) {
@@ -868,8 +873,11 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
p = &s->isr.reg_u32[offset];
break;
case EIOINTC_COREISR_START ... EIOINTC_COREISR_END:
+ if (cpu >= s->num_cpu)
+ return -EINVAL;
+
offset = (addr - EIOINTC_COREISR_START) / 4;
- p = &s->coreisr.reg_u32[cpuid][offset];
+ p = &s->coreisr.reg_u32[cpu][offset];
break;
case EIOINTC_COREMAP_START ... EIOINTC_COREMAP_END:
offset = (addr - EIOINTC_COREMAP_START) / 4;
--
2.39.3
