linux-kernel - Re: [PATCH v3 4/9] LoongArch: KVM: INTC: Check validation of num

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAAhV-H7ehdkKwzsFNAaX+r5eXLknvskyXLPDKei2A55LoSiJMA@mail.gmail.com>
Date: Thu, 19 Jun 2025 16:46:59 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: Bibo Mao <maobibo@...ngson.cn>
Cc: Tianrui Zhao <zhaotianrui@...ngson.cn>, Xianglai Li <lixianglai@...ngson.cn>, kvm@...r.kernel.org, 
	loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org, 
	stable@...r.kernel.org
Subject: Re: [PATCH v3 4/9] LoongArch: KVM: INTC: Check validation of num_cpu
 from user space

Hi, Bibo,

On Wed, Jun 11, 2025 at 9:47 AM Bibo Mao <maobibo@...ngson.cn> 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@...r.kernel.org
> Fixes: 1ad7efa552fd ("LoongArch: KVM: Add EIOINTC user mode read and write functions")
> Signed-off-by: Bibo Mao <maobibo@...ngson.cn>
> ---
>  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.


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
>