[PATCH] target/hppa: prevent overflow in BTLB entry size calculation

[PATCH] target/hppa: prevent overflow in BTLB entry size calculation

[gerben]

From: Denis Rastyogin <gerben@altlinux.org>

Cast len to long long before multiplying by TARGET_PAGE_SIZE
when calculating btlb->itree.last to ensure 64-bit arithmetic
and avoid potential overflow.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Denis Rastyogin <gerben@altlinux.org>
---
 target/hppa/mem_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index 9bdd0a6f23..0c196b5bfc 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -766,7 +766,7 @@ void HELPER(diag_btlb)(CPUHPPAState *env)
 
             /* Create new BTLB entry */
             btlb->itree.start = virt_page << TARGET_PAGE_BITS;
-            btlb->itree.last = btlb->itree.start + len * TARGET_PAGE_SIZE - 1;
+            btlb->itree.last = btlb->itree.start + (long long) len * TARGET_PAGE_SIZE - 1;
             btlb->pa = phys_page << TARGET_PAGE_BITS;
             set_access_bits_pa11(env, btlb, env->gr[20]);
             btlb->t = 0;
-- 
2.42.2

Re: [PATCH] target/hppa: prevent overflow in BTLB entry size calculation

[Richard Henderson]

On 7/22/25 03:18, gerben@altlinux.org wrote:
> From: Denis Rastyogin <gerben@altlinux.org>
> 
> Cast len to long long before multiplying by TARGET_PAGE_SIZE
> when calculating btlb->itree.last to ensure 64-bit arithmetic
> and avoid potential overflow.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Denis Rastyogin <gerben@altlinux.org>
> ---
>   target/hppa/mem_helper.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
> index 9bdd0a6f23..0c196b5bfc 100644
> --- a/target/hppa/mem_helper.c
> +++ b/target/hppa/mem_helper.c
> @@ -766,7 +766,7 @@ void HELPER(diag_btlb)(CPUHPPAState *env)
>   
>               /* Create new BTLB entry */
>               btlb->itree.start = virt_page << TARGET_PAGE_BITS;
> -            btlb->itree.last = btlb->itree.start + len * TARGET_PAGE_SIZE - 1;
> +            btlb->itree.last = btlb->itree.start + (long long) len * TARGET_PAGE_SIZE - 1;
>               btlb->pa = phys_page << TARGET_PAGE_BITS;
>               set_access_bits_pa11(env, btlb, env->gr[20]);
>               btlb->t = 0;

(1) long long is always wrong.

(2) If there's truncation anywhere, it's in the type of len itself:

       unsigned int len; len = env->gpr[21];

     However, from the comment at the top of the function I deduce
     this is a parisc-1.1 thing, where gprs are 32 bits, so this is
     producing the correct result.


r~

[PATCH] target/hppa: prevent overflow in BTLB entry size calculation

[gerben]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 1155 bytes --]

1) I’m not entirely sure why using long long is considered “always wrong,”
especially since just a few lines above there’s a similar usage here:

qemu_log_mask(CPU_LOG_MMU, "PDC_BLOCK_TLB: PDC_BTLB_INSERT "
                    "0x%08llx-0x%08llx: vpage 0x%llx for phys page 0x%04x len %d "
                    "into slot %d\n",
                    (long long) virt_page << TARGET_PAGE_BITS,
                    (long long) (virt_page + len) << TARGET_PAGE_BITS,
                    (long long) virt_page, phys_page, len, slot);

That said, I do agree that using long long here might not be the best approach, and I’ll fix it.

2) If len can approach INT32_MAX, why wouldn’t the calculation len * TARGET_PAGE_SIZE cause an overflow?
This operation is done between an unsigned int and a constant, and uses 32-bit arithmetic.
I agree this patch likely doesn’t affect real scenarios — when running the Debian image
I saw only values where len <= 2048 and TARGET_PAGE_SIZE = 4096, so no overflow would occur.
However, as I understand it, these values come from outside and are not validated anywhere,
so theoretically they could be arbitrary.

Re: [PATCH] target/hppa: prevent overflow in BTLB entry size calculation

[Richard Henderson]

On 8/14/25 20:43, gerben@altlinux.org wrote:
> 1) I’m not entirely sure why using long long is considered “always wrong,”
> especially since just a few lines above there’s a similar usage here:

Because "long" has host-specific length, we eschew it completely in favor of types with 
definite length like "int32_t" or "int64_t".  Because we prefer int64_t, there is zero 
benefit to using "long long".

Just because there are other mis-uses of "long" or "long long" doesn't mean we should 
introduce more.

> 2) If len can approach INT32_MAX, why wouldn’t the calculation len * TARGET_PAGE_SIZE cause an overflow?

Because we're computing a 32-bit value for pa1.1, a 32-bit guest.  If the guest asks for a 
nonsense computation, it should get it.


r~