[PATCH 1/1] target/arm: Fix arithmetic underflow in SETM instruction

[PATCH 1/1] target/arm: Fix arithmetic underflow in SETM instruction

[Ido Plat]

Pass the stage size to step function callback,
otherwise do_setm would hang when size is larger then page size because stage size would underflow.
This fix changes do_setm to be more inline with do_setp.

Fixes: 0e92818887dee ("target/arm: Implement the SET* instructions")
Signed-off-by: Ido Plat <ido.plat1@ibm.com>
---
 target/arm/tcg/helper-a64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
index 56b431faf5..8f42a28d07 100644
--- a/target/arm/tcg/helper-a64.c
+++ b/target/arm/tcg/helper-a64.c
@@ -1348,7 +1348,7 @@ static void do_setm(CPUARMState *env, uint32_t syndrome, uint32_t mtedesc,
     /* Do the actual memset: we leave the last partial page to SETE */
     stagesetsize = setsize & TARGET_PAGE_MASK;
     while (stagesetsize > 0) {
-        step = stepfn(env, toaddr, setsize, data, memidx, &mtedesc, ra);
+        step = stepfn(env, toaddr, stagesetsize, data, memidx, &mtedesc, ra);
         toaddr += step;
         setsize -= step;
         stagesetsize -= step;
-- 
2.43.0

Re: [PATCH 1/1] target/arm: Fix arithmetic underflow in SETM instruction

[Richard Henderson]

On 10/25/24 03:48, Ido Plat wrote:
> Pass the stage size to step function callback,
> otherwise do_setm would hang when size is larger then page size because stage size would underflow.
> This fix changes do_setm to be more inline with do_setp.
> 
> Fixes: 0e92818887dee ("target/arm: Implement the SET* instructions")
> Signed-off-by: Ido Plat<ido.plat1@ibm.com>
> ---
>   target/arm/tcg/helper-a64.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)


Fixes: 0e92818887d ("target/arm: Implement the SET* instructions")
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 1/1] target/arm: Fix arithmetic underflow in SETM instruction

[Michael Tokarev]

25.10.2024 05:48, Ido Plat wrote:
> Pass the stage size to step function callback,
> otherwise do_setm would hang when size is larger then page size because stage size would underflow.
> This fix changes do_setm to be more inline with do_setp.
> 
> Fixes: 0e92818887dee ("target/arm: Implement the SET* instructions")
> Signed-off-by: Ido Plat <ido.plat1@ibm.com>
> ---
>   target/arm/tcg/helper-a64.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
> index 56b431faf5..8f42a28d07 100644
> --- a/target/arm/tcg/helper-a64.c
> +++ b/target/arm/tcg/helper-a64.c
> @@ -1348,7 +1348,7 @@ static void do_setm(CPUARMState *env, uint32_t syndrome, uint32_t mtedesc,
>       /* Do the actual memset: we leave the last partial page to SETE */
>       stagesetsize = setsize & TARGET_PAGE_MASK;
>       while (stagesetsize > 0) {
> -        step = stepfn(env, toaddr, setsize, data, memidx, &mtedesc, ra);
> +        step = stepfn(env, toaddr, stagesetsize, data, memidx, &mtedesc, ra);
>           toaddr += step;
>           setsize -= step;
>           stagesetsize -= step;
This also seems to be:

Cc: qemu-stable@nongnu.org

(Cc'ed now, there's no need to add it again).

Please let me know if it shouldn't.

Thanks,

/mjt

Re: [PATCH 1/1] target/arm: Fix arithmetic underflow in SETM instruction

[Peter Maydell]

On Fri, 25 Oct 2024 at 16:50, Michael Tokarev <mjt@tls.msk.ru> wrote:
>
> 25.10.2024 05:48, Ido Plat wrote:
> > Pass the stage size to step function callback,
> > otherwise do_setm would hang when size is larger then page size because stage size would underflow.
> > This fix changes do_setm to be more inline with do_setp.
> >
> > Fixes: 0e92818887dee ("target/arm: Implement the SET* instructions")
> > Signed-off-by: Ido Plat <ido.plat1@ibm.com>
> > ---
> >   target/arm/tcg/helper-a64.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
> > index 56b431faf5..8f42a28d07 100644
> > --- a/target/arm/tcg/helper-a64.c
> > +++ b/target/arm/tcg/helper-a64.c
> > @@ -1348,7 +1348,7 @@ static void do_setm(CPUARMState *env, uint32_t syndrome, uint32_t mtedesc,
> >       /* Do the actual memset: we leave the last partial page to SETE */
> >       stagesetsize = setsize & TARGET_PAGE_MASK;
> >       while (stagesetsize > 0) {
> > -        step = stepfn(env, toaddr, setsize, data, memidx, &mtedesc, ra);
> > +        step = stepfn(env, toaddr, stagesetsize, data, memidx, &mtedesc, ra);
> >           toaddr += step;
> >           setsize -= step;
> >           stagesetsize -= step;
> This also seems to be:
>
> Cc: qemu-stable@nongnu.org
>
> (Cc'ed now, there's no need to add it again).
>
> Please let me know if it shouldn't.

Yes, I think it's stable material.



Applied to target-arm.next, thanks.

-- PMM