mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr()
@ 2026-03-02  8:53 Jiakai Xu
  2026-03-02 12:41 ` Anup Patel
  2026-03-03  4:44 ` jk.hong
  0 siblings, 2 replies; 4+ messages in thread
From: Jiakai Xu @ 2026-03-02  8:53 UTC (permalink / raw)
  To: linux-kernel, linux-riscv, kvm-riscv, kvm
  Cc: Alexandre Ghiti, Albert Ou, Palmer Dabbelt, Paul Walmsley,
	Atish Patra, Anup Patel, Jiakai Xu, Jiakai Xu

Fuzzer reports a KASAN use-after-free bug triggered by a race
between KVM_HAS_DEVICE_ATTR and KVM_SET_DEVICE_ATTR ioctls on the AIA
device. The root cause is that aia_has_attr() invokes
kvm_riscv_aia_aplic_has_attr() without holding dev->kvm->lock, while
a concurrent aia_set_attr() may call aia_init() under that lock. When
aia_init() fails after kvm_riscv_aia_aplic_init() has succeeded, it
calls kvm_riscv_aia_aplic_cleanup() in its fail_cleanup_imsics path,
which frees both aplic_state and aplic_state->irqs. The concurrent
has_attr path can then dereference the freed aplic->irqs in
aplic_read_pending():
	irqd = &aplic->irqs[irq];   /* UAF here */

KASAN report:
 BUG: KASAN: slab-use-after-free in aplic_read_pending
             arch/riscv/kvm/aia_aplic.c:119 [inline]
 BUG: KASAN: slab-use-after-free in aplic_read_pending_word
             arch/riscv/kvm/aia_aplic.c:351 [inline]
 BUG: KASAN: slab-use-after-free in aplic_mmio_read_offset
             arch/riscv/kvm/aia_aplic.c:406
 Read of size 8 at addr ff600000ba965d58 by task 9498
 Call Trace:
  aplic_read_pending arch/riscv/kvm/aia_aplic.c:119 [inline]
  aplic_read_pending_word arch/riscv/kvm/aia_aplic.c:351 [inline]
  aplic_mmio_read_offset arch/riscv/kvm/aia_aplic.c:406
  kvm_riscv_aia_aplic_has_attr arch/riscv/kvm/aia_aplic.c:566
  aia_has_attr arch/riscv/kvm/aia_device.c:469
 allocated by task 9473:
  kvm_riscv_aia_aplic_init arch/riscv/kvm/aia_aplic.c:583
  aia_init arch/riscv/kvm/aia_device.c:248 [inline]
  aia_set_attr arch/riscv/kvm/aia_device.c:334
 freed by task 9473:
  kvm_riscv_aia_aplic_cleanup arch/riscv/kvm/aia_aplic.c:644
  aia_init arch/riscv/kvm/aia_device.c:292 [inline]
  aia_set_attr arch/riscv/kvm/aia_device.c:334

Fix this race by acquiring dev->kvm->lock in aia_has_attr() before
calling kvm_riscv_aia_aplic_has_attr(), consistent with the locking
pattern used in aia_get_attr() and aia_set_attr().

Fixes: 289a007b98b06d ("RISC-V: KVM: Expose APLIC registers as attributes of AIA irqchip")
Signed-off-by: Jiakai Xu <jiakaiPeanut@gmail.com>
Signed-off-by: Jiakai Xu <xujiakai2025@iscas.ac.cn>
---
V1 -> V2:
- Fixed the race by adding locking in aia_has_attr() instead of
  introducing a new validation function, as suggested by Anup Patel.
---
 arch/riscv/kvm/aia_device.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/riscv/kvm/aia_device.c b/arch/riscv/kvm/aia_device.c
index b195a93add1ce..ef944d7097d29 100644
--- a/arch/riscv/kvm/aia_device.c
+++ b/arch/riscv/kvm/aia_device.c
@@ -466,7 +466,9 @@ static int aia_has_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 		}
 		break;
 	case KVM_DEV_RISCV_AIA_GRP_APLIC:
+		mutex_lock(&dev->kvm->lock);
 		return kvm_riscv_aia_aplic_has_attr(dev->kvm, attr->attr);
+		mutex_unlock(&dev->kvm->lock);
 	case KVM_DEV_RISCV_AIA_GRP_IMSIC:
 		return kvm_riscv_aia_imsic_has_attr(dev->kvm, attr->attr);
 	}
-- 
2.34.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr()
  2026-03-02  8:53 [PATCH v2] RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr() Jiakai Xu
@ 2026-03-02 12:41 ` Anup Patel
  2026-03-03  4:44 ` jk.hong
  1 sibling, 0 replies; 4+ messages in thread
From: Anup Patel @ 2026-03-02 12:41 UTC (permalink / raw)
  To: Jiakai Xu
  Cc: linux-kernel, linux-riscv, kvm-riscv, kvm, Alexandre Ghiti,
	Albert Ou, Palmer Dabbelt, Paul Walmsley, Atish Patra, Jiakai Xu

On Mon, Mar 2, 2026 at 2:23 PM Jiakai Xu <xujiakai2025@iscas.ac.cn> wrote:
>
> Fuzzer reports a KASAN use-after-free bug triggered by a race
> between KVM_HAS_DEVICE_ATTR and KVM_SET_DEVICE_ATTR ioctls on the AIA
> device. The root cause is that aia_has_attr() invokes
> kvm_riscv_aia_aplic_has_attr() without holding dev->kvm->lock, while
> a concurrent aia_set_attr() may call aia_init() under that lock. When
> aia_init() fails after kvm_riscv_aia_aplic_init() has succeeded, it
> calls kvm_riscv_aia_aplic_cleanup() in its fail_cleanup_imsics path,
> which frees both aplic_state and aplic_state->irqs. The concurrent
> has_attr path can then dereference the freed aplic->irqs in
> aplic_read_pending():
>         irqd = &aplic->irqs[irq];   /* UAF here */
>
> KASAN report:
>  BUG: KASAN: slab-use-after-free in aplic_read_pending
>              arch/riscv/kvm/aia_aplic.c:119 [inline]
>  BUG: KASAN: slab-use-after-free in aplic_read_pending_word
>              arch/riscv/kvm/aia_aplic.c:351 [inline]
>  BUG: KASAN: slab-use-after-free in aplic_mmio_read_offset
>              arch/riscv/kvm/aia_aplic.c:406
>  Read of size 8 at addr ff600000ba965d58 by task 9498
>  Call Trace:
>   aplic_read_pending arch/riscv/kvm/aia_aplic.c:119 [inline]
>   aplic_read_pending_word arch/riscv/kvm/aia_aplic.c:351 [inline]
>   aplic_mmio_read_offset arch/riscv/kvm/aia_aplic.c:406
>   kvm_riscv_aia_aplic_has_attr arch/riscv/kvm/aia_aplic.c:566
>   aia_has_attr arch/riscv/kvm/aia_device.c:469
>  allocated by task 9473:
>   kvm_riscv_aia_aplic_init arch/riscv/kvm/aia_aplic.c:583
>   aia_init arch/riscv/kvm/aia_device.c:248 [inline]
>   aia_set_attr arch/riscv/kvm/aia_device.c:334
>  freed by task 9473:
>   kvm_riscv_aia_aplic_cleanup arch/riscv/kvm/aia_aplic.c:644
>   aia_init arch/riscv/kvm/aia_device.c:292 [inline]
>   aia_set_attr arch/riscv/kvm/aia_device.c:334
>
> Fix this race by acquiring dev->kvm->lock in aia_has_attr() before
> calling kvm_riscv_aia_aplic_has_attr(), consistent with the locking
> pattern used in aia_get_attr() and aia_set_attr().
>
> Fixes: 289a007b98b06d ("RISC-V: KVM: Expose APLIC registers as attributes of AIA irqchip")
> Signed-off-by: Jiakai Xu <jiakaiPeanut@gmail.com>
> Signed-off-by: Jiakai Xu <xujiakai2025@iscas.ac.cn>
> ---
> V1 -> V2:
> - Fixed the race by adding locking in aia_has_attr() instead of
>   introducing a new validation function, as suggested by Anup Patel.
> ---
>  arch/riscv/kvm/aia_device.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/riscv/kvm/aia_device.c b/arch/riscv/kvm/aia_device.c
> index b195a93add1ce..ef944d7097d29 100644
> --- a/arch/riscv/kvm/aia_device.c
> +++ b/arch/riscv/kvm/aia_device.c
> @@ -466,7 +466,9 @@ static int aia_has_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
>                 }
>                 break;
>         case KVM_DEV_RISCV_AIA_GRP_APLIC:
> +               mutex_lock(&dev->kvm->lock);
>                 return kvm_riscv_aia_aplic_has_attr(dev->kvm, attr->attr);
> +               mutex_unlock(&dev->kvm->lock);

Need to do the following here:

    mutex_lock(&dev->kvm->lock);
    ret = kvm_riscv_aia_aplic_has_attr(dev->kvm, attr->attr);
    mutex_unlock(&dev->kvm->lock);
    return ret;

Regards,
Anup

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr()
  2026-03-02  8:53 [PATCH v2] RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr() Jiakai Xu
  2026-03-02 12:41 ` Anup Patel
@ 2026-03-03  4:44 ` jk.hong
  2026-03-03  5:56   ` Jiakai Xu
  1 sibling, 1 reply; 4+ messages in thread
From: jk.hong @ 2026-03-03  4:44 UTC (permalink / raw)
  To: xujiakai2025; +Cc: linux-kernel, linux-riscv, kvm-riscv, Ryan JK Hong

From: Ryan JK Hong <ryan.jk.hong@gmail.com>

> Fuzzer reports a KASAN use-after-free bug triggered by a race
> between KVM_HAS_DEVICE_ATTR and KVM_SET_DEVICE_ATTR ioctls on the AIA
> device. The root cause is that aia_has_attr() invokes
> kvm_riscv_aia_aplic_has_attr() without holding dev->kvm->lock, while
> a concurrent aia_set_attr() may call aia_init() under that lock. When
> aia_init() fails after kvm_riscv_aia_aplic_init() has succeeded, it
> calls kvm_riscv_aia_aplic_cleanup() in its fail_cleanup_imsics path,
> which frees both aplic_state and aplic_state->irqs. The concurrent
> has_attr path can then dereference the freed aplic->irqs in
> aplic_read_pending():
> 	irqd = &aplic->irqs[irq];   /* UAF here */
> 
> KASAN report:
>   BUG: KASAN: slab-use-after-free in aplic_read_pending
>               arch/riscv/kvm/aia_aplic.c:119 [inline]
>   BUG: KASAN: slab-use-after-free in aplic_read_pending_word
>               arch/riscv/kvm/aia_aplic.c:351 [inline]
>   BUG: KASAN: slab-use-after-free in aplic_mmio_read_offset
>               arch/riscv/kvm/aia_aplic.c:406
>   Read of size 8 at addr ff600000ba965d58 by task 9498
>   Call Trace:
>    aplic_read_pending arch/riscv/kvm/aia_aplic.c:119 [inline]
>    aplic_read_pending_word arch/riscv/kvm/aia_aplic.c:351 [inline]
>    aplic_mmio_read_offset arch/riscv/kvm/aia_aplic.c:406
>    kvm_riscv_aia_aplic_has_attr arch/riscv/kvm/aia_aplic.c:566
>    aia_has_attr arch/riscv/kvm/aia_device.c:469
>   allocated by task 9473:
>    kvm_riscv_aia_aplic_init arch/riscv/kvm/aia_aplic.c:583
>    aia_init arch/riscv/kvm/aia_device.c:248 [inline]
>    aia_set_attr arch/riscv/kvm/aia_device.c:334
>   freed by task 9473:
>    kvm_riscv_aia_aplic_cleanup arch/riscv/kvm/aia_aplic.c:644
>    aia_init arch/riscv/kvm/aia_device.c:292 [inline]
>    aia_set_attr arch/riscv/kvm/aia_device.c:334
> 
> Fix this race by acquiring dev->kvm->lock in aia_has_attr() before
> calling kvm_riscv_aia_aplic_has_attr(), consistent with the locking
> pattern used in aia_get_attr() and aia_set_attr().
> 
> Fixes: 289a007b98b06d ("RISC-V: KVM: Expose APLIC registers as attributes of AIA irqchip")
> Signed-off-by: Jiakai Xu<jiakaiPeanut@gmail.com>
> Signed-off-by: Jiakai Xu<xujiakai2025@iscas.ac.cn>
> ---
> V2 -> V3:
> - Fixed incorrect locking pattern in aia_has_attr(): avoid returning
>    while holding dev->kvm->lock by storing the return value in a local
>    variable, unlocking, and then returning.
> V1 -> V2:
> - Fixed the race by adding locking in aia_has_attr() instead of
>    introducing a new validation function, as suggested by Anup Patel.
> ---
>   arch/riscv/kvm/aia_device.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/kvm/aia_device.c b/arch/riscv/kvm/aia_device.c
> index b195a93add1ce..0722cbaed5ec9 100644
> --- a/arch/riscv/kvm/aia_device.c
> +++ b/arch/riscv/kvm/aia_device.c
> @@ -437,7 +437,7 @@ static int aia_get_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
>   
>   static int aia_has_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
>   {
> -	int nr_vcpus;
> +	int nr_vcpus, r = -ENXIO;
>   
>   	switch (attr->group) {
>   	case KVM_DEV_RISCV_AIA_GRP_CONFIG:
> @@ -466,12 +466,15 @@ static int aia_has_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
>   		}
>   		break;
>   	case KVM_DEV_RISCV_AIA_GRP_APLIC:
> -		return kvm_riscv_aia_aplic_has_attr(dev->kvm, attr->attr);
> +		mutex_lock(&dev->kvm->lock);
> +		r = kvm_riscv_aia_aplic_has_attr(dev->kvm, attr->attr);
> +		mutex_unlock(&dev->kvm->lock);
> +		return r;
>   	case KVM_DEV_RISCV_AIA_GRP_IMSIC:
>   		return kvm_riscv_aia_imsic_has_attr(dev->kvm, attr->attr);
>   	}
>   
> -	return -ENXIO;
> +	return r;
>   }
>   
>   struct kvm_device_ops kvm_riscv_aia_device_ops = {

This case branch has the same problem.
	case KVM_DEV_RISCV_AIA_GRP_IMSIC:
		mutex_lock(&dev->kvm->lock);
		ret = kvm_riscv_aia_imsic_has_attr(dev->kvm, attr->attr);
		mutex_unlock(&dev->kvm->lock);
		return ret;
	}


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re:[PATCH v2] RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr()
  2026-03-03  4:44 ` jk.hong
@ 2026-03-03  5:56   ` Jiakai Xu
  0 siblings, 0 replies; 4+ messages in thread
From: Jiakai Xu @ 2026-03-03  5:56 UTC (permalink / raw)
  To: ryan.jk.hong; +Cc: kvm-riscv, linux-kernel, linux-riscv, xujiakai2025

Hi Ryan,

Thank you for pointing this out.

I did notice that the IMSIC branch follows a similar pattern. However, 
I am planning to address the KVM_DEV_RISCV_AIA_GRP_IMSIC case in a 
separate commit to keep this fix focused strictly on the reported APLIC 
use-after-free issue.

I will send a follow-up patch to add the corresponding locking for the 
IMSIC branch.

Thanks again for the careful review.

Best regards,
Jiakai


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-03-03  5:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-02  8:53 [PATCH v2] RISC-V: KVM: Fix use-after-free in kvm_riscv_aia_aplic_has_attr() Jiakai Xu
2026-03-02 12:41 ` Anup Patel
2026-03-03  4:44 ` jk.hong
2026-03-03  5:56   ` Jiakai Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®