[PATCH] RISC-V: KVM: Fix out-of-bounds index into aia_irq2bitpos[]

[PATCH] RISC-V: KVM: Fix out-of-bounds index into aia_irq2bitpos[]

[Wang Yechao]

aia_get_iprio8() and aia_set_iprio8() index the 64-entry
aia_irq2bitpos[] array with an irq value derived from the
guest-controlled VSISELECT register. For RV64:

  first_irq = (isel - ISELECT_IPRIO0) * 4
  nirqs = 8
  isel up to ISELECT_IPRIO15

so the irq number can reach 67 and aia_irq2bitpos[64..67] is
accessed out of bounds. A malicious guest can trigger this,
causing host panic under KASAN/UBSAN or undefined behavior
otherwise.

Add an explicit bounds check in both accessors before indexing
the table.

Fixes: 2f4d58f7635a ("RISC-V: KVM: Virtualize per-HART AIA CSRs")
Signed-off-by: Wang Yechao <wang.yechao255@zte.com.cn>
---
 arch/riscv/kvm/aia.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/kvm/aia.c b/arch/riscv/kvm/aia.c
index b3b466e4db33..a51d2db7f331 100644
--- a/arch/riscv/kvm/aia.c
+++ b/arch/riscv/kvm/aia.c
@@ -286,8 +286,12 @@ static int aia_irq2bitpos[] = {
 static u8 aia_get_iprio8(struct kvm_vcpu *vcpu, unsigned int irq)
 {
 	unsigned long hviprio;
-	int bitpos = aia_irq2bitpos[irq];
+	int bitpos;
 
+	if (irq >= ARRAY_SIZE(aia_irq2bitpos))
+		return 0;
+
+	bitpos = aia_irq2bitpos[irq];
 	if (bitpos < 0)
 		return 0;
 
@@ -319,8 +323,12 @@ static u8 aia_get_iprio8(struct kvm_vcpu *vcpu, unsigned int irq)
 static void aia_set_iprio8(struct kvm_vcpu *vcpu, unsigned int irq, u8 prio)
 {
 	unsigned long hviprio;
-	int bitpos = aia_irq2bitpos[irq];
+	int bitpos;
+
+	if (irq >= ARRAY_SIZE(aia_irq2bitpos))
+		return;
 
+	bitpos = aia_irq2bitpos[irq];
 	if (bitpos < 0)
 		return;
 
-- 
2.27.0

Re: [PATCH] RISC-V: KVM: Fix out-of-bounds index into aia_irq2bitpos[]

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] riscv: kvm: dead bounds check in aia_get_iprio8() and aia_set_iprio8()
--

commit c874cd5cc69d874febde23d82ec7ecdefc1600f2
Author: Wang Yechao <wang.yechao255@zte.com.cn>

RISC-V: KVM: Fix out-of-bounds index into aia_irq2bitpos[]

This commit adds bounds checks against ARRAY_SIZE(aia_irq2bitpos) in
aia_get_iprio8() and aia_set_iprio8() to prevent an alleged out-of-bounds
array access when emulating per-HART AIA priority registers on RV64.

>     RISC-V: KVM: Fix out-of-bounds index into aia_irq2bitpos[]
>     
>     aia_get_iprio8() and aia_set_iprio8() index the 64-entry
>     aia_irq2bitpos[] array with an irq value derived from the
>     guest-controlled VSISELECT register. For RV64:
>     
>       first_irq = (isel - ISELECT_IPRIO0) * 4
>       nirqs = 8
>       isel up to ISELECT_IPRIO15
>     
>     so the irq number can reach 67 and aia_irq2bitpos[64..67] is
>     accessed out of bounds. A malicious guest can trigger this,
>     causing host panic under KASAN/UBSAN or undefined behavior
>     otherwise.
>     
>     Add an explicit bounds check in both accessors before indexing
>     the table.
>     
>     Fixes: 2f4d58f7635a ("RISC-V: KVM: Virtualize per-HART AIA CSRs")
>     Signed-off-by: Wang Yechao <wang.yechao255@zte.com.cn>
>
> diff --git a/arch/riscv/kvm/aia.c b/arch/riscv/kvm/aia.c
> index b3b466e4db337..a51d2db7f3311 100644
> --- a/arch/riscv/kvm/aia.c
> +++ b/arch/riscv/kvm/aia.c
> @@ -286,8 +286,12 @@ static int aia_irq2bitpos[] = {
>  static u8 aia_get_iprio8(struct kvm_vcpu *vcpu, unsigned int irq)
>  {
>  	unsigned long hviprio;
> -	int bitpos = aia_irq2bitpos[irq];
> +	int bitpos;
>  
> +	if (irq >= ARRAY_SIZE(aia_irq2bitpos))
> +		return 0;
> +
> +	bitpos = aia_irq2bitpos[irq];
>  	if (bitpos < 0)
>  		return 0;
>  
> @@ -319,8 +323,12 @@ static u8 aia_get_iprio8(struct kvm_vcpu *vcpu, unsigned int irq)
>  static void aia_set_iprio8(struct kvm_vcpu *vcpu, unsigned int irq, u8 prio)
>  {
>  	unsigned long hviprio;
> -	int bitpos = aia_irq2bitpos[irq];
> +	int bitpos;
> +
> +	if (irq >= ARRAY_SIZE(aia_irq2bitpos))
> +		return;

[Severity: Medium]
Can irq actually reach 64..67 in aia_get_iprio8() or aia_set_iprio8()?

Both helpers are static and only called from aia_rmw_iprio() via
kvm_riscv_vcpu_aia_rmw_ireg() when isel is between ISELECT_IPRIO0 (0x30) and
ISELECT_IPRIO15 (0x3f).

In arch/riscv/kvm/aia.c, aia_rmw_iprio() rejects odd isel values on RV64
before computing first_irq:

arch/riscv/kvm/aia.c:aia_rmw_iprio() {
#ifndef CONFIG_32BIT
	if (isel & 0x1)
		return KVM_INSN_ILLEGAL_TRAP;
#endif

	nirqs = 4 * (BITS_PER_LONG / 32);
	first_irq = (isel - ISELECT_IPRIO0) * 4;
    ...
}

Because ISELECT_IPRIO15 (0x3f) is odd, it returns KVM_INSN_ILLEGAL_TRAP on
RV64. The largest even isel reaching the loop on RV64 is ISELECT_IPRIO14
(0x3e), which gives first_irq = (0x3e - 0x30) * 4 = 56 and nirqs = 8, so
first_irq + i is at most 56 + 7 = 63. On RV32, isel can be ISELECT_IPRIO15
(0x3f), which gives first_irq = 60 and nirqs = 4, so first_irq + i is also
at most 60 + 3 = 63.

Since ARRAY_SIZE(aia_irq2bitpos) is 64, isn't irq already guaranteed to be
in 0..63, making these bounds checks unreachable?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027114439.699273-1-wang.yechao255@zte.com.cn?part=1

Re: [PATCH] RISC-V: KVM: Fix out-of-bounds index into aia_irq2bitpos[]

[Wang Yechao]

> [Severity: Medium]
> Can irq actually reach 64..67 in aia_get_iprio8() or aia_set_iprio8()?
> 
> Both helpers are static and only called from aia_rmw_iprio() via
> kvm_riscv_vcpu_aia_rmw_ireg() when isel is between ISELECT_IPRIO0 (0x30) and
> ISELECT_IPRIO15 (0x3f).
> 
> In arch/riscv/kvm/aia.c, aia_rmw_iprio() rejects odd isel values on RV64
> before computing first_irq:
> 
> arch/riscv/kvm/aia.c:aia_rmw_iprio() {
> #ifndef CONFIG_32BIT
>     if (isel & 0x1)
>         return KVM_INSN_ILLEGAL_TRAP;
> #endif
> 
>     nirqs = 4 * (BITS_PER_LONG / 32);
>     first_irq = (isel - ISELECT_IPRIO0) * 4;
>     ...
> }
> 
> Because ISELECT_IPRIO15 (0x3f) is odd, it returns KVM_INSN_ILLEGAL_TRAP on
> RV64. The largest even isel reaching the loop on RV64 is ISELECT_IPRIO14
> (0x3e), which gives first_irq = (0x3e - 0x30) * 4 = 56 and nirqs = 8, so
> first_irq + i is at most 56 + 7 = 63. On RV32, isel can be ISELECT_IPRIO15
> (0x3f), which gives first_irq = 60 and nirqs = 4, so first_irq + i is also
> at most 60 + 3 = 63.
> 
> Since ARRAY_SIZE(aia_irq2bitpos) is 64, isn't irq already guaranteed to be
> in 0..63, making these bounds checks unreachable?

Right,

I missed the odd-isel check in aia_rmw_iprio(). 
Please drop this patch. 

Thanks,
Yechao