[Qemu-devel] [PATCH 00/10] Avoid integer overflow in next_page_start

[Qemu-devel] [PATCH 00/10] Avoid integer overflow in next_page_start

[Emilio G. Cota]

Richard pointed out in another thread that when computing
next_page_start we can break checks for the last page in the
address space due to integer overflow. This affects several targets;
the appended fixes them.

You can fetch the patches from:
  https://github.com/cota/qemu/tree/next_page_overflow

Thanks,

		Emilio
---
 target/arm/translate.c        | 11 +++++------
 target/arm/translate.h        |  2 +-
 target/cris/translate.c       |  6 +++---
 target/lm32/translate.c       |  6 +++---
 target/microblaze/translate.c |  6 +++---
 target/mips/translate.c       |  6 +++---
 target/riscv/translate.c      |  6 +++---
 target/s390x/translate.c      |  6 +++---
 target/tilegx/translate.c     |  4 ++--
 target/unicore32/translate.c  |  6 +++---
 target/xtensa/translate.c     |  9 ++++-----
 11 files changed, 33 insertions(+), 35 deletions(-)

[Qemu-devel] [PATCH 01/10] target/riscv: avoid integer overflow in next_page PC check

[Emilio G. Cota]

If the PC is in the last page of the address space, next_page_start
overflows to 0. Fix it.

Reported-by: Richard Henderson <richard.henderson@linaro.org>
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Cc: Michael Clark <mjc@sifive.com>
Cc: Palmer Dabbelt <palmer@sifive.com>
Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 target/riscv/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 808eab7..d2d2e5e 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1849,11 +1849,11 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb)
     CPURISCVState *env = cs->env_ptr;
     DisasContext ctx;
     target_ulong pc_start;
-    target_ulong next_page_start;
+    target_ulong page_start;
     int num_insns;
     int max_insns;
     pc_start = tb->pc;
-    next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+    page_start = pc_start & TARGET_PAGE_MASK;
     ctx.pc = pc_start;
 
     /* once we have GDB, the rest of the translate.c implementation should be
@@ -1903,7 +1903,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb)
         if (cs->singlestep_enabled) {
             break;
         }
-        if (ctx.pc >= next_page_start) {
+        if (ctx.pc - page_start >= TARGET_PAGE_SIZE) {
             break;
         }
         if (tcg_op_buf_full()) {
-- 
2.7.4

[Qemu-devel] [PATCH 02/10] target/cris: avoid integer overflow in next_page PC check

[Emilio G. Cota]

If the PC is in the last page of the address space, next_page_start
overflows to 0. Fix it.

Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 target/cris/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/cris/translate.c b/target/cris/translate.c
index f51a731..64b9ec6 100644
--- a/target/cris/translate.c
+++ b/target/cris/translate.c
@@ -3091,7 +3091,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
     unsigned int insn_len;
     struct DisasContext ctx;
     struct DisasContext *dc = &ctx;
-    uint32_t next_page_start;
+    uint32_t page_start;
     target_ulong npc;
     int num_insns;
     int max_insns;
@@ -3138,7 +3138,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
 
     dc->cpustate_changed = 0;
 
-    next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+    page_start = pc_start & TARGET_PAGE_MASK;
     num_insns = 0;
     max_insns = tb_cflags(tb) & CF_COUNT_MASK;
     if (max_insns == 0) {
@@ -3234,7 +3234,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
     } while (!dc->is_jmp && !dc->cpustate_changed
             && !tcg_op_buf_full()
             && !singlestep
-            && (dc->pc < next_page_start)
+            && (dc->pc - page_start < TARGET_PAGE_SIZE)
             && num_insns < max_insns);
 
     if (dc->clear_locked_irq) {
-- 
2.7.4

[Qemu-devel] [PATCH 03/10] target/lm32: avoid integer overflow in next_page PC check

[Emilio G. Cota]

If the PC is in the last page of the address space, next_page_start
overflows to 0. Fix it.

Cc: Michael Walle <michael@walle.cc>
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 target/lm32/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/lm32/translate.c b/target/lm32/translate.c
index 2e1c5e6..fdd206a 100644
--- a/target/lm32/translate.c
+++ b/target/lm32/translate.c
@@ -1055,7 +1055,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
     LM32CPU *cpu = lm32_env_get_cpu(env);
     struct DisasContext ctx, *dc = &ctx;
     uint32_t pc_start;
-    uint32_t next_page_start;
+    uint32_t page_start;
     int num_insns;
     int max_insns;
 
@@ -1075,7 +1075,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
         pc_start &= ~3;
     }
 
-    next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+    page_start = pc_start & TARGET_PAGE_MASK;
     num_insns = 0;
     max_insns = tb_cflags(tb) & CF_COUNT_MASK;
     if (max_insns == 0) {
@@ -1115,7 +1115,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
          && !tcg_op_buf_full()
          && !cs->singlestep_enabled
          && !singlestep
-         && (dc->pc < next_page_start)
+         && (dc->pc - page_start < TARGET_PAGE_SIZE)
          && num_insns < max_insns);
 
     if (tb_cflags(tb) & CF_LAST_IO) {
-- 
2.7.4

[Qemu-devel] [PATCH 04/10] target/xtensa: avoid integer overflow in next_page PC check

[Emilio G. Cota]

If the PC is in the last page of the address space, next_page_start
overflows to 0. Fix it.

Cc: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 target/xtensa/translate.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index 4f6d030..aad4963 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -1061,8 +1061,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb)
     int insn_count = 0;
     int max_insns = tb_cflags(tb) & CF_COUNT_MASK;
     uint32_t pc_start = tb->pc;
-    uint32_t next_page_start =
-        (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+    uint32_t page_start = pc_start & TARGET_PAGE_MASK;
 
     if (max_insns == 0) {
         max_insns = CF_COUNT_MASK;
@@ -1162,9 +1161,9 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb)
         }
     } while (dc.is_jmp == DISAS_NEXT &&
             insn_count < max_insns &&
-            dc.pc < next_page_start &&
-            dc.pc + xtensa_insn_len(env, &dc) <= next_page_start &&
-            !tcg_op_buf_full());
+            dc.pc - page_start < TARGET_PAGE_SIZE &&
+            dc.pc - page_start + xtensa_insn_len(env, &dc) <= TARGET_PAGE_SIZE
+            && !tcg_op_buf_full());
 done:
     reset_sar_tracker(&dc);
     if (dc.icount) {
-- 
2.7.4

[Qemu-devel] [PATCH 05/10] target/unicore32: avoid integer overflow in next_page PC check

[Emilio G. Cota]

If the PC is in the last page of the address space, next_page_start
overflows to 0. Fix it.

Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 target/unicore32/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/unicore32/translate.c b/target/unicore32/translate.c
index 5b51f21..abe2ea8 100644
--- a/target/unicore32/translate.c
+++ b/target/unicore32/translate.c
@@ -1875,7 +1875,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb)
     CPUUniCore32State *env = cs->env_ptr;
     DisasContext dc1, *dc = &dc1;
     target_ulong pc_start;
-    uint32_t next_page_start;
+    uint32_t page_start;
     int num_insns;
     int max_insns;
 
@@ -1894,7 +1894,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb)
     cpu_F1s = tcg_temp_new_i32();
     cpu_F0d = tcg_temp_new_i64();
     cpu_F1d = tcg_temp_new_i64();
-    next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+    page_start = pc_start & TARGET_PAGE_MASK;
     num_insns = 0;
     max_insns = tb_cflags(tb) & CF_COUNT_MASK;
     if (max_insns == 0) {
@@ -1951,7 +1951,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb)
     } while (!dc->is_jmp && !tcg_op_buf_full() &&
              !cs->singlestep_enabled &&
              !singlestep &&
-             dc->pc < next_page_start &&
+             dc->pc - page_start < TARGET_PAGE_SIZE &&
              num_insns < max_insns);
 
     if (tb_cflags(tb) & CF_LAST_IO) {
-- 
2.7.4

[Qemu-devel] [PATCH 06/10] target/tilegx: avoid integer overflow in next_page PC check

[Emilio G. Cota]

If the PC is in the last page of the address space, next_page_start
overflows to 0. Fix it.

Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 target/tilegx/translate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/tilegx/translate.c b/target/tilegx/translate.c
index d63bf5b..6c53c5e 100644
--- a/target/tilegx/translate.c
+++ b/target/tilegx/translate.c
@@ -2375,7 +2375,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
     DisasContext ctx;
     DisasContext *dc = &ctx;
     uint64_t pc_start = tb->pc;
-    uint64_t next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+    uint64_t page_start = pc_start & TARGET_PAGE_MASK;
     int num_insns = 0;
     int max_insns = tb_cflags(tb) & CF_COUNT_MASK;
 
@@ -2415,7 +2415,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
         }
         dc->pc += TILEGX_BUNDLE_SIZE_IN_BYTES;
         if (num_insns >= max_insns
-            || dc->pc >= next_page_start
+            || (dc->pc - page_start >= TARGET_PAGE_SIZE)
             || tcg_op_buf_full()) {
             /* Ending the TB due to TB size or page boundary.  Set PC.  */
             tcg_gen_movi_tl(cpu_pc, dc->pc);
-- 
2.7.4

[Qemu-devel] [PATCH 07/10] target/microblaze: avoid integer overflow in next_page PC check

[Emilio G. Cota]

If the PC is in the last page of the address space, next_page_start
overflows to 0. Fix it.

Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 target/microblaze/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 7628b0e..401dbe6 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -1637,7 +1637,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
     uint32_t pc_start;
     struct DisasContext ctx;
     struct DisasContext *dc = &ctx;
-    uint32_t next_page_start, org_flags;
+    uint32_t page_start, org_flags;
     target_ulong npc;
     int num_insns;
     int max_insns;
@@ -1663,7 +1663,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
         cpu_abort(cs, "Microblaze: unaligned PC=%x\n", pc_start);
     }
 
-    next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+    page_start = pc_start & TARGET_PAGE_MASK;
     num_insns = 0;
     max_insns = tb_cflags(tb) & CF_COUNT_MASK;
     if (max_insns == 0) {
@@ -1749,7 +1749,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
     } while (!dc->is_jmp && !dc->cpustate_changed
              && !tcg_op_buf_full()
              && !singlestep
-             && (dc->pc < next_page_start)
+             && (dc->pc - page_start < TARGET_PAGE_SIZE)
              && num_insns < max_insns);
 
     npc = dc->pc;
-- 
2.7.4

[Qemu-devel] [PATCH 08/10] target/arm: avoid integer overflow in next_page PC check

[Emilio G. Cota]

If the PC is in the last page of the address space, next_page_start
overflows to 0. Fix it.

Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 target/arm/translate.h |  2 +-
 target/arm/translate.c | 11 +++++------
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/target/arm/translate.h b/target/arm/translate.h
index c47febf..2287894 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -9,7 +9,7 @@ typedef struct DisasContext {
     DisasContextBase base;
 
     target_ulong pc;
-    target_ulong next_page_start;
+    target_ulong page_start;
     uint32_t insn;
     /* Nonzero if this instruction has been conditionally skipped.  */
     int condjmp;
diff --git a/target/arm/translate.c b/target/arm/translate.c
index fc03b5b..ade8d2d 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -9913,7 +9913,7 @@ static bool thumb_insn_is_16bit(DisasContext *s, uint32_t insn)
         return false;
     }
 
-    if ((insn >> 11) == 0x1e && (s->pc < s->next_page_start - 3)) {
+    if ((insn >> 11) == 0x1e && s->pc - s->page_start < TARGET_PAGE_SIZE - 3) {
         /* 0b1111_0xxx_xxxx_xxxx : BL/BLX prefix, and the suffix
          * is not on the next page; we merge this into a 32-bit
          * insn.
@@ -12269,8 +12269,7 @@ static int arm_tr_init_disas_context(DisasContextBase *dcbase,
     dc->is_ldex = false;
     dc->ss_same_el = false; /* Can't be true since EL_d must be AArch64 */
 
-    dc->next_page_start =
-        (dc->base.pc_first & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+    dc->page_start = dc->base.pc_first & TARGET_PAGE_MASK;
 
     /* If architectural single step active, limit to 1.  */
     if (is_singlestepping(dc)) {
@@ -12280,7 +12279,7 @@ static int arm_tr_init_disas_context(DisasContextBase *dcbase,
     /* ARM is a fixed-length ISA.  Bound the number of insns to execute
        to those left on the page.  */
     if (!dc->thumb) {
-        int bound = (dc->next_page_start - dc->base.pc_first) / 4;
+        int bound = -(dc->base.pc_first | TARGET_PAGE_MASK) / 4;
         max_insns = MIN(max_insns, bound);
     }
 
@@ -12552,8 +12551,8 @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
      * but isn't very efficient).
      */
     if (dc->base.is_jmp == DISAS_NEXT
-        && (dc->pc >= dc->next_page_start
-            || (dc->pc >= dc->next_page_start - 3
+        && (dc->pc - dc->page_start >= TARGET_PAGE_SIZE
+            || (dc->pc - dc->page_start >= TARGET_PAGE_SIZE - 3
                 && insn_crosses_page(env, dc)))) {
         dc->base.is_jmp = DISAS_TOO_MANY;
     }
-- 
2.7.4

[Qemu-devel] [PATCH 09/10] target/s390x: avoid integer overflow in next_page PC check

[Emilio G. Cota]

If the PC is in the last page of the address space, next_page_start
overflows to 0. Fix it.

Cc: Cornelia Huck <cohuck@redhat.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: David Hildenbrand <david@redhat.com>
Cc: qemu-s390x@nongnu.org
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 target/s390x/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 7d39ab3..44449f1 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -6163,7 +6163,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
     CPUS390XState *env = cs->env_ptr;
     DisasContext dc;
     target_ulong pc_start;
-    uint64_t next_page_start;
+    uint64_t page_start;
     int num_insns, max_insns;
     ExitStatus status;
     bool do_debug;
@@ -6181,7 +6181,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
     dc.ex_value = tb->cs_base;
     do_debug = dc.singlestep_enabled = cs->singlestep_enabled;
 
-    next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+    page_start = pc_start & TARGET_PAGE_MASK;
 
     num_insns = 0;
     max_insns = tb_cflags(tb) & CF_COUNT_MASK;
@@ -6218,7 +6218,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
         /* If we reach a page boundary, are single stepping,
            or exhaust instruction count, stop generation.  */
         if (status == NO_EXIT
-            && (dc.pc >= next_page_start
+            && (dc.pc - page_start >= TARGET_PAGE_SIZE
                 || tcg_op_buf_full()
                 || num_insns >= max_insns
                 || singlestep
-- 
2.7.4

[Qemu-devel] [PATCH 10/10] target/mips: avoid integer overflow in next_page PC check

[Emilio G. Cota]

If the PC is in the last page of the address space, next_page_start
overflows to 0. Fix it.

Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Yongbok Kim <yongbok.kim@mips.com>
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 target/mips/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index d05ee67..d8e717d 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -20202,14 +20202,14 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
     CPUMIPSState *env = cs->env_ptr;
     DisasContext ctx;
     target_ulong pc_start;
-    target_ulong next_page_start;
+    target_ulong page_start;
     int num_insns;
     int max_insns;
     int insn_bytes;
     int is_slot;
 
     pc_start = tb->pc;
-    next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+    page_start = pc_start & TARGET_PAGE_MASK;
     ctx.pc = pc_start;
     ctx.saved_pc = -1;
     ctx.singlestep_enabled = cs->singlestep_enabled;
@@ -20320,7 +20320,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
             break;
         }
 
-        if (ctx.pc >= next_page_start) {
+        if (ctx.pc - page_start >= TARGET_PAGE_SIZE) {
             break;
         }
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH 04/10] target/xtensa: avoid integer overflow in next_page PC check

[Max Filippov]

On Tue, Apr 10, 2018 at 9:19 AM, Emilio G. Cota <cota@braap.org> wrote:
> If the PC is in the last page of the address space, next_page_start
> overflows to 0. Fix it.
>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> ---
>  target/xtensa/translate.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)

Acked-by: Max Filippov <jcmvbkbc@gmail.com>

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 00/10] Avoid integer overflow in next_page_start

[Richard Henderson]

On 04/11/2018 02:19 AM, Emilio G. Cota wrote:
> Richard pointed out in another thread that when computing
> next_page_start we can break checks for the last page in the
> address space due to integer overflow. This affects several targets;
> the appended fixes them.
> 
> You can fetch the patches from:
>   https://github.com/cota/qemu/tree/next_page_overflow

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [qemu-s390x] [PATCH 09/10] target/s390x: avoid integer overflow in next_page PC check

[Thomas Huth]

On 10.04.2018 18:19, Emilio G. Cota wrote:
> If the PC is in the last page of the address space, next_page_start
> overflows to 0. Fix it.
> 
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: qemu-s390x@nongnu.org
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> ---
>  target/s390x/translate.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 7d39ab3..44449f1 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -6163,7 +6163,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
>      CPUS390XState *env = cs->env_ptr;
>      DisasContext dc;
>      target_ulong pc_start;
> -    uint64_t next_page_start;
> +    uint64_t page_start;
>      int num_insns, max_insns;
>      ExitStatus status;
>      bool do_debug;
> @@ -6181,7 +6181,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
>      dc.ex_value = tb->cs_base;
>      do_debug = dc.singlestep_enabled = cs->singlestep_enabled;
>  
> -    next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
> +    page_start = pc_start & TARGET_PAGE_MASK;
>  
>      num_insns = 0;
>      max_insns = tb_cflags(tb) & CF_COUNT_MASK;
> @@ -6218,7 +6218,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
>          /* If we reach a page boundary, are single stepping,
>             or exhaust instruction count, stop generation.  */
>          if (status == NO_EXIT
> -            && (dc.pc >= next_page_start
> +            && (dc.pc - page_start >= TARGET_PAGE_SIZE
>                  || tcg_op_buf_full()
>                  || num_insns >= max_insns
>                  || singlestep
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH 03/10] target/lm32: avoid integer overflow in next_page PC check

[Michael Walle]

Am 2018-04-10 18:19, schrieb Emilio G. Cota:
> If the PC is in the last page of the address space, next_page_start
> overflows to 0. Fix it.
> 
> Cc: Michael Walle <michael@walle.cc>
> Signed-off-by: Emilio G. Cota <cota@braap.org>

Acked-by: Michael Walle <michael@walle.cc>

Re: [Qemu-devel] [PATCH 09/10] target/s390x: avoid integer overflow in next_page PC check

[David Hildenbrand]

On 10.04.2018 18:19, Emilio G. Cota wrote:
> If the PC is in the last page of the address space, next_page_start
> overflows to 0. Fix it.
> 
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: qemu-s390x@nongnu.org
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> ---
>  target/s390x/translate.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 7d39ab3..44449f1 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -6163,7 +6163,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
>      CPUS390XState *env = cs->env_ptr;
>      DisasContext dc;
>      target_ulong pc_start;
> -    uint64_t next_page_start;
> +    uint64_t page_start;
>      int num_insns, max_insns;
>      ExitStatus status;
>      bool do_debug;
> @@ -6181,7 +6181,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
>      dc.ex_value = tb->cs_base;
>      do_debug = dc.singlestep_enabled = cs->singlestep_enabled;
>  
> -    next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
> +    page_start = pc_start & TARGET_PAGE_MASK;
>  
>      num_insns = 0;
>      max_insns = tb_cflags(tb) & CF_COUNT_MASK;
> @@ -6218,7 +6218,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
>          /* If we reach a page boundary, are single stepping,
>             or exhaust instruction count, stop generation.  */
>          if (status == NO_EXIT
> -            && (dc.pc >= next_page_start
> +            && (dc.pc - page_start >= TARGET_PAGE_SIZE
>                  || tcg_op_buf_full()
>                  || num_insns >= max_insns
>                  || singlestep
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 00/10] Avoid integer overflow in next_page_start

[Emilio G. Cota]

On Wed, Apr 11, 2018 at 10:08:58 +1000, Richard Henderson wrote:
> On 04/11/2018 02:19 AM, Emilio G. Cota wrote:
> > Richard pointed out in another thread that when computing
> > next_page_start we can break checks for the last page in the
> > address space due to integer overflow. This affects several targets;
> > the appended fixes them.
> > 
> > You can fetch the patches from:
> >   https://github.com/cota/qemu/tree/next_page_overflow
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Thanks!

To ease an eventual merge I'll be updating the patches' R-b tags as
they come in this branch:
  https://github.com/cota/qemu/tree/next_page_overflow-r-b

BTW to avoid conflicts we should merge this before the translator loop
conversion series; I'll make that clear when I send a new version
of that patch set.

		Emilio

Re: [Qemu-devel] [PATCH 00/10] Avoid integer overflow in next_page_start

[Cornelia Huck]

On Wed, 11 Apr 2018 11:29:42 -0400
"Emilio G. Cota" <cota@braap.org> wrote:

> On Wed, Apr 11, 2018 at 10:08:58 +1000, Richard Henderson wrote:
> > On 04/11/2018 02:19 AM, Emilio G. Cota wrote:  
> > > Richard pointed out in another thread that when computing
> > > next_page_start we can break checks for the last page in the
> > > address space due to integer overflow. This affects several targets;
> > > the appended fixes them.
> > > 
> > > You can fetch the patches from:
> > >   https://github.com/cota/qemu/tree/next_page_overflow  
> > 
> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>  
> 
> Thanks!
> 
> To ease an eventual merge I'll be updating the patches' R-b tags as
> they come in this branch:
>   https://github.com/cota/qemu/tree/next_page_overflow-r-b
> 
> BTW to avoid conflicts we should merge this before the translator loop
> conversion series; I'll make that clear when I send a new version
> of that patch set.
> 
> 		Emilio

So, this series will be merged in one go, then? I'll ack the s390x
patch.

Re: [Qemu-devel] [PATCH 09/10] target/s390x: avoid integer overflow in next_page PC check

[Cornelia Huck]

On Tue, 10 Apr 2018 12:19:45 -0400
"Emilio G. Cota" <cota@braap.org> wrote:

> If the PC is in the last page of the address space, next_page_start
> overflows to 0. Fix it.
> 
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: qemu-s390x@nongnu.org
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> ---
>  target/s390x/translate.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Acked-by: Cornelia Huck <cohuck@redhat.com>

Re: [Qemu-devel] [PATCH 01/10] target/riscv: avoid integer overflow in next_page PC check

[Bastian Koppelmann]

On 04/10/2018 06:19 PM, Emilio G. Cota wrote:
> If the PC is in the last page of the address space, next_page_start
> overflows to 0. Fix it.
> 
> Reported-by: Richard Henderson <richard.henderson@linaro.org>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Cc: Michael Clark <mjc@sifive.com>
> Cc: Palmer Dabbelt <palmer@sifive.com>
> Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> ---

Acked-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>

Cheers,
Bastian

Re: [Qemu-devel] [PATCH 01/10] target/riscv: avoid integer overflow in next_page PC check

[Michael Clark]

On Wed, Apr 11, 2018 at 4:19 AM, Emilio G. Cota <cota@braap.org> wrote:

> If the PC is in the last page of the address space, next_page_start
> overflows to 0. Fix it.
>
> Reported-by: Richard Henderson <richard.henderson@linaro.org>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Cc: Michael Clark <mjc@sifive.com>
> Cc: Palmer Dabbelt <palmer@sifive.com>
> Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Signed-off-by: Emilio G. Cota <cota@braap.org>
>

Reviewed-by: Michael Clark <mjc@sifive.com>


> ---
>  target/riscv/translate.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 808eab7..d2d2e5e 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -1849,11 +1849,11 @@ void gen_intermediate_code(CPUState *cs,
> TranslationBlock *tb)
>      CPURISCVState *env = cs->env_ptr;
>      DisasContext ctx;
>      target_ulong pc_start;
> -    target_ulong next_page_start;
> +    target_ulong page_start;
>      int num_insns;
>      int max_insns;
>      pc_start = tb->pc;
> -    next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
> +    page_start = pc_start & TARGET_PAGE_MASK;
>      ctx.pc = pc_start;
>
>      /* once we have GDB, the rest of the translate.c implementation
> should be
> @@ -1903,7 +1903,7 @@ void gen_intermediate_code(CPUState *cs,
> TranslationBlock *tb)
>          if (cs->singlestep_enabled) {
>              break;
>          }
> -        if (ctx.pc >= next_page_start) {
> +        if (ctx.pc - page_start >= TARGET_PAGE_SIZE) {
>              break;
>          }
>          if (tcg_op_buf_full()) {
> --
> 2.7.4
>
>

Re: [Qemu-devel] [PATCH 00/10] Avoid integer overflow in next_page_start

[Richard Henderson]

On 04/12/2018 01:29 AM, Emilio G. Cota wrote:
> To ease an eventual merge I'll be updating the patches' R-b tags as
> they come in this branch:
>   https://github.com/cota/qemu/tree/next_page_overflow-r-b
> 
> BTW to avoid conflicts we should merge this before the translator loop
> conversion series; I'll make that clear when I send a new version
> of that patch set.

Right-o.  Thanks.  We'll get these in right away once development starts again
so that you don't have to carry it long.


r~

Re: [Qemu-devel] [PATCH 00/10] Avoid integer overflow in next_page_start

[Michael Clark]

On Thu, Apr 12, 2018 at 11:56 AM, Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 04/12/2018 01:29 AM, Emilio G. Cota wrote:
> > To ease an eventual merge I'll be updating the patches' R-b tags as
> > they come in this branch:
> >   https://github.com/cota/qemu/tree/next_page_overflow-r-b
> >
> > BTW to avoid conflicts we should merge this before the translator loop
> > conversion series; I'll make that clear when I send a new version
> > of that patch set.
>
> Right-o.  Thanks.  We'll get these in right away once development starts
> again
> so that you don't have to carry it long.


Emilio,

Is this your latest branch for these changes?

- https://github.com/cota/qemu/tree/trloop-conv-v3

I see that this branch has this change "target/riscv: avoid integer
overflow in next_page PC check"

I have some changes for target/riscv/translate.c and was thinking of basing
them on your patches so we don't have conflicts.

Thanks,
Michael

Re: [Qemu-devel] [PATCH 00/10] Avoid integer overflow in next_page_start

[Emilio G. Cota]

On Wed, May 09, 2018 at 12:51:14 +1200, Michael Clark wrote:
> Emilio,
> 
> Is this your latest branch for these changes?
> 
> - https://github.com/cota/qemu/tree/trloop-conv-v3
> 
> I see that this branch has this change "target/riscv: avoid integer
> overflow in next_page PC check"
> 
> I have some changes for target/riscv/translate.c and was thinking of basing
> them on your patches so we don't have conflicts.

Yes that's the latest branch. Richard is currently working
on a pull request to get that branch merged, so you'll soon
be able to rebase your changes on top of master.

Thanks,

		Emilio