[PATCH] target/riscv/pmp: Fix integer overflow in TOR and NA4 address computation

Zishun Yi posted 1 patch 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260511071347.3000476-1-vulab@iscas.ac.cn
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Chao Liu <chao.liu.zevorn@gmail.com>
There is a newer version of this series
target/riscv/pmp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH] target/riscv/pmp: Fix integer overflow in TOR and NA4 address computation
Posted by Zishun Yi 2 weeks ago
According to the RISC-V Privileged Manual: "The Sv32 page-based
virtual-memory scheme described in sv32 supports 34-bit physical
addresses for RV32, so the PMP scheme must support addresses wider than
XLEN for RV32."

However, the current QEMU implementation uses `target_ulong` (which
resolves to `uint32_t` on RV32) for PMP address variables.  When
shifting these addresses left (e.g., `this_addr << 2`), an integer
overflow occurs, truncating the high bits of the 34-bit physical
address.

Fix this issue by casting the `target_ulong` variables to `hwaddr`
before performing the left shift operation.

This issue was discovered and reported by SpecHunter, an AI-driven
architecture specification analysis tool.

Link:https://github.com/yizishun/rv-isa-sec/blob/master/output/riscv-isa-manual/pr-2472/qemu.txt
Signed-off-by: Zishun Yi <vulab@iscas.ac.cn>
---
 target/riscv/pmp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 5391caa59c7d..dfddafcbcb48 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -253,12 +253,12 @@ void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index)
             sa = ea = 0u;
             break;
         }
-        sa = prev_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
-        ea = (this_addr << 2) - 1u;
+        sa = (hwaddr)prev_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
+        ea = ((hwaddr)this_addr << 2) - 1u;
         break;
 
     case PMP_AMATCH_NA4:
-        sa = this_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
+        sa = (hwaddr)this_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
         ea = (sa + 4u) - 1u;
         break;
 
-- 
2.51.2
Re: [PATCH] target/riscv/pmp: Fix integer overflow in TOR and NA4 address computation
Posted by Daniel Henrique Barboza 2 weeks ago

On 5/11/2026 4:13 AM, Zishun Yi wrote:
> According to the RISC-V Privileged Manual: "The Sv32 page-based
> virtual-memory scheme described in sv32 supports 34-bit physical
> addresses for RV32, so the PMP scheme must support addresses wider than
> XLEN for RV32."
> 
> However, the current QEMU implementation uses `target_ulong` (which
> resolves to `uint32_t` on RV32) for PMP address variables.  When
> shifting these addresses left (e.g., `this_addr << 2`), an integer
> overflow occurs, truncating the high bits of the 34-bit physical
> address.
> 
> Fix this issue by casting the `target_ulong` variables to `hwaddr`
> before performing the left shift operation.
> 
> This issue was discovered and reported by SpecHunter, an AI-driven
> architecture specification analysis tool.
> 
> Link:https://github.com/yizishun/rv-isa-sec/blob/master/output/riscv-isa-manual/pr-2472/qemu.txt
> Signed-off-by: Zishun Yi <vulab@iscas.ac.cn>
> ---
>   target/riscv/pmp.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 5391caa59c7d..dfddafcbcb48 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -253,12 +253,12 @@ void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index)
>               sa = ea = 0u;
>               break;
>           }
> -        sa = prev_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
> -        ea = (this_addr << 2) - 1u;
> +        sa = (hwaddr)prev_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
> +        ea = ((hwaddr)this_addr << 2) - 1u;


Instead of casting 'hwaddr' I'd rather change both to 'hwaddr' type.  The
new type suits both variables, and in fact pmp_decode_napot() expects a
hwaddr:

pmp_decode_napot(this_addr, &sa, &ea);



Thanks,
Daniel


>           break;
>   
>       case PMP_AMATCH_NA4:
> -        sa = this_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
> +        sa = (hwaddr)this_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
>           ea = (sa + 4u) - 1u;
>           break;
>
[PATCH v2] target/riscv/pmp: Fix integer overflow in TOR and NA4 address computation
Posted by Zishun Yi 2 weeks ago
According to the RISC-V Privileged Manual: "The Sv32 page-based
virtual-memory scheme described in sv32 supports 34-bit physical
addresses for RV32, so the PMP scheme must support addresses wider than
XLEN for RV32."

However, the current QEMU implementation uses `target_ulong` (which
resolves to `uint32_t` on RV32) for PMP address variables.  When
shifting these addresses left (e.g., `this_addr << 2`), an integer
overflow occurs, truncating the high bits of the 34-bit physical
address.

Fix this issue by changing the types of PMP address variables
(`this_addr` and `prev_addr`) to `hwaddr`.

This issue was discovered and reported by SpecHunter, an AI-driven
architecture specification analysis tool.

Link: https://github.com/yizishun/rv-isa-sec/blob/master/output/riscv-isa-manual/pr-2472/qemu.txt
Signed-off-by: Zishun Yi <vulab@iscas.ac.cn>
---
Changes in v2:
- Changed the types of `this_addr` and `prev_addr` to `hwaddr` instead
  of using inline casting, by Daniel's suggestion.

 target/riscv/pmp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 5391caa59c7d..a71091a316e0 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -227,8 +227,8 @@ static void pmp_decode_napot(hwaddr a, hwaddr *sa, hwaddr *ea)
 void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index)
 {
     uint8_t this_cfg = env->pmp_state.pmp[pmp_index].cfg_reg;
-    target_ulong this_addr = env->pmp_state.pmp[pmp_index].addr_reg;
-    target_ulong prev_addr = 0u;
+    hwaddr this_addr = env->pmp_state.pmp[pmp_index].addr_reg;
+    hwaddr prev_addr = 0u;
     hwaddr sa = 0u;
     hwaddr ea = 0u;
     int g = pmp_get_granularity_g(env);
-- 
2.51.2
Re: [PATCH v2] target/riscv/pmp: Fix integer overflow in TOR and NA4 address computation
Posted by Alistair Francis 1 week ago
On Mon, May 11, 2026 at 8:27 PM Zishun Yi <vulab@iscas.ac.cn> wrote:
>
> According to the RISC-V Privileged Manual: "The Sv32 page-based
> virtual-memory scheme described in sv32 supports 34-bit physical
> addresses for RV32, so the PMP scheme must support addresses wider than
> XLEN for RV32."
>
> However, the current QEMU implementation uses `target_ulong` (which
> resolves to `uint32_t` on RV32) for PMP address variables.  When
> shifting these addresses left (e.g., `this_addr << 2`), an integer
> overflow occurs, truncating the high bits of the 34-bit physical
> address.
>
> Fix this issue by changing the types of PMP address variables
> (`this_addr` and `prev_addr`) to `hwaddr`.
>
> This issue was discovered and reported by SpecHunter, an AI-driven
> architecture specification analysis tool.
>
> Link: https://github.com/yizishun/rv-isa-sec/blob/master/output/riscv-isa-manual/pr-2472/qemu.txt
> Signed-off-by: Zishun Yi <vulab@iscas.ac.cn>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
> Changes in v2:
> - Changed the types of `this_addr` and `prev_addr` to `hwaddr` instead
>   of using inline casting, by Daniel's suggestion.
>
>  target/riscv/pmp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 5391caa59c7d..a71091a316e0 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -227,8 +227,8 @@ static void pmp_decode_napot(hwaddr a, hwaddr *sa, hwaddr *ea)
>  void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index)
>  {
>      uint8_t this_cfg = env->pmp_state.pmp[pmp_index].cfg_reg;
> -    target_ulong this_addr = env->pmp_state.pmp[pmp_index].addr_reg;
> -    target_ulong prev_addr = 0u;
> +    hwaddr this_addr = env->pmp_state.pmp[pmp_index].addr_reg;
> +    hwaddr prev_addr = 0u;
>      hwaddr sa = 0u;
>      hwaddr ea = 0u;
>      int g = pmp_get_granularity_g(env);
> --
> 2.51.2
>
>
Re: [PATCH v2] target/riscv/pmp: Fix integer overflow in TOR and NA4 address computation
Posted by Alistair Francis 1 week ago
On Mon, May 11, 2026 at 8:27 PM Zishun Yi <vulab@iscas.ac.cn> wrote:
>
> According to the RISC-V Privileged Manual: "The Sv32 page-based
> virtual-memory scheme described in sv32 supports 34-bit physical
> addresses for RV32, so the PMP scheme must support addresses wider than
> XLEN for RV32."
>
> However, the current QEMU implementation uses `target_ulong` (which
> resolves to `uint32_t` on RV32) for PMP address variables.  When
> shifting these addresses left (e.g., `this_addr << 2`), an integer
> overflow occurs, truncating the high bits of the 34-bit physical
> address.
>
> Fix this issue by changing the types of PMP address variables
> (`this_addr` and `prev_addr`) to `hwaddr`.
>
> This issue was discovered and reported by SpecHunter, an AI-driven
> architecture specification analysis tool.
>
> Link: https://github.com/yizishun/rv-isa-sec/blob/master/output/riscv-isa-manual/pr-2472/qemu.txt
> Signed-off-by: Zishun Yi <vulab@iscas.ac.cn>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> Changes in v2:
> - Changed the types of `this_addr` and `prev_addr` to `hwaddr` instead
>   of using inline casting, by Daniel's suggestion.
>
>  target/riscv/pmp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 5391caa59c7d..a71091a316e0 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -227,8 +227,8 @@ static void pmp_decode_napot(hwaddr a, hwaddr *sa, hwaddr *ea)
>  void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index)
>  {
>      uint8_t this_cfg = env->pmp_state.pmp[pmp_index].cfg_reg;
> -    target_ulong this_addr = env->pmp_state.pmp[pmp_index].addr_reg;
> -    target_ulong prev_addr = 0u;
> +    hwaddr this_addr = env->pmp_state.pmp[pmp_index].addr_reg;
> +    hwaddr prev_addr = 0u;
>      hwaddr sa = 0u;
>      hwaddr ea = 0u;
>      int g = pmp_get_granularity_g(env);
> --
> 2.51.2
>
>
Re: [PATCH v2] target/riscv/pmp: Fix integer overflow in TOR and NA4 address computation
Posted by Daniel Henrique Barboza 2 weeks ago

On 5/11/2026 7:26 AM, Zishun Yi wrote:
> According to the RISC-V Privileged Manual: "The Sv32 page-based
> virtual-memory scheme described in sv32 supports 34-bit physical
> addresses for RV32, so the PMP scheme must support addresses wider than
> XLEN for RV32."
> 
> However, the current QEMU implementation uses `target_ulong` (which
> resolves to `uint32_t` on RV32) for PMP address variables.  When
> shifting these addresses left (e.g., `this_addr << 2`), an integer
> overflow occurs, truncating the high bits of the 34-bit physical
> address.
> 
> Fix this issue by changing the types of PMP address variables
> (`this_addr` and `prev_addr`) to `hwaddr`.
> 
> This issue was discovered and reported by SpecHunter, an AI-driven
> architecture specification analysis tool.
> 
> Link: https://github.com/yizishun/rv-isa-sec/blob/master/output/riscv-isa-manual/pr-2472/qemu.txt
> Signed-off-by: Zishun Yi <vulab@iscas.ac.cn>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

> Changes in v2:
> - Changed the types of `this_addr` and `prev_addr` to `hwaddr` instead
>    of using inline casting, by Daniel's suggestion.
> 
>   target/riscv/pmp.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 5391caa59c7d..a71091a316e0 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -227,8 +227,8 @@ static void pmp_decode_napot(hwaddr a, hwaddr *sa, hwaddr *ea)
>   void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index)
>   {
>       uint8_t this_cfg = env->pmp_state.pmp[pmp_index].cfg_reg;
> -    target_ulong this_addr = env->pmp_state.pmp[pmp_index].addr_reg;
> -    target_ulong prev_addr = 0u;
> +    hwaddr this_addr = env->pmp_state.pmp[pmp_index].addr_reg;
> +    hwaddr prev_addr = 0u;
>       hwaddr sa = 0u;
>       hwaddr ea = 0u;
>       int g = pmp_get_granularity_g(env);