| Message ID | 20210414134112.25620-2-iii@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | accel/tcg: Make sure that tb->size != 0 after translation | expand |
On 14.04.21 15:41, Ilya Leoshkevich wrote: > Hitting an uretprobe in a s390x TCG guest causes a SIGSEGV. What > happens is: > > * uretprobe maps a userspace page containing an invalid instruction. > * uretprobe replaces the target function's return address with the > address of that page. > * When tb_gen_code() is called on that page, tb->size ends up being 0 > (because the page starts with the invalid instruction), which causes > virt_page2 to point to the previous page. > * The previous page is not mapped, so this causes a spurious > translation exception. > > tb->size must never be 0: even if there is an illegal instruction, the > instruction bytes that have been looked at must count towards tb->size. > So adjust s390x's translate_one() to act this way for both illegal > instructions and instructions that are known to generate exceptions. > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- > target/s390x/translate.c | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/target/s390x/translate.c b/target/s390x/translate.c > index 4f953ddfba..e243624d2a 100644 > --- a/target/s390x/translate.c > +++ b/target/s390x/translate.c > @@ -6412,7 +6412,8 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s) > qemu_log_mask(LOG_UNIMP, "unimplemented opcode 0x%02x%02x\n", > s->fields.op, s->fields.op2); > gen_illegal_opcode(s); > - return DISAS_NORETURN; > + ret = DISAS_NORETURN; > + goto out; > } > > #ifndef CONFIG_USER_ONLY > @@ -6428,7 +6429,8 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s) > /* privileged instruction */ > if ((s->base.tb->flags & FLAG_MASK_PSTATE) && (insn->flags & IF_PRIV)) { > gen_program_exception(s, PGM_PRIVILEGED); > - return DISAS_NORETURN; > + ret = DISAS_NORETURN; > + goto out; > } > > /* if AFP is not enabled, instructions and registers are forbidden */ > @@ -6455,7 +6457,8 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s) > } > if (dxc) { > gen_data_exception(dxc); > - return DISAS_NORETURN; > + ret = DISAS_NORETURN; > + goto out; > } > } > > @@ -6463,7 +6466,8 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s) > if (insn->flags & IF_VEC) { > if (!((s->base.tb->flags & FLAG_MASK_VECTOR))) { > gen_data_exception(0xfe); > - return DISAS_NORETURN; > + ret = DISAS_NORETURN; > + goto out; > } > } > > @@ -6484,7 +6488,8 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s) > (insn->spec & SPEC_r1_f128 && !is_fp_pair(get_field(s, r1))) || > (insn->spec & SPEC_r2_f128 && !is_fp_pair(get_field(s, r2)))) { > gen_program_exception(s, PGM_SPECIFICATION); > - return DISAS_NORETURN; > + ret = DISAS_NORETURN; > + goto out; > } > } > > @@ -6544,6 +6549,7 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s) > } > #endif > > +out: > /* Advance to the next instruction. */ > s->base.pc_next = s->pc_tmp; > return ret; > Reviewed-by: David Hildenbrand <david@redhat.com>
diff --git a/target/s390x/translate.c b/target/s390x/translate.c index 4f953ddfba..e243624d2a 100644 --- a/target/s390x/translate.c +++ b/target/s390x/translate.c @@ -6412,7 +6412,8 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s) qemu_log_mask(LOG_UNIMP, "unimplemented opcode 0x%02x%02x\n", s->fields.op, s->fields.op2); gen_illegal_opcode(s); - return DISAS_NORETURN; + ret = DISAS_NORETURN; + goto out; } #ifndef CONFIG_USER_ONLY @@ -6428,7 +6429,8 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s) /* privileged instruction */ if ((s->base.tb->flags & FLAG_MASK_PSTATE) && (insn->flags & IF_PRIV)) { gen_program_exception(s, PGM_PRIVILEGED); - return DISAS_NORETURN; + ret = DISAS_NORETURN; + goto out; } /* if AFP is not enabled, instructions and registers are forbidden */ @@ -6455,7 +6457,8 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s) } if (dxc) { gen_data_exception(dxc); - return DISAS_NORETURN; + ret = DISAS_NORETURN; + goto out; } } @@ -6463,7 +6466,8 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s) if (insn->flags & IF_VEC) { if (!((s->base.tb->flags & FLAG_MASK_VECTOR))) { gen_data_exception(0xfe); - return DISAS_NORETURN; + ret = DISAS_NORETURN; + goto out; } } @@ -6484,7 +6488,8 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s) (insn->spec & SPEC_r1_f128 && !is_fp_pair(get_field(s, r1))) || (insn->spec & SPEC_r2_f128 && !is_fp_pair(get_field(s, r2)))) { gen_program_exception(s, PGM_SPECIFICATION); - return DISAS_NORETURN; + ret = DISAS_NORETURN; + goto out; } } @@ -6544,6 +6549,7 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s) } #endif +out: /* Advance to the next instruction. */ s->base.pc_next = s->pc_tmp; return ret;
Hitting an uretprobe in a s390x TCG guest causes a SIGSEGV. What happens is: * uretprobe maps a userspace page containing an invalid instruction. * uretprobe replaces the target function's return address with the address of that page. * When tb_gen_code() is called on that page, tb->size ends up being 0 (because the page starts with the invalid instruction), which causes virt_page2 to point to the previous page. * The previous page is not mapped, so this causes a spurious translation exception. tb->size must never be 0: even if there is an illegal instruction, the instruction bytes that have been looked at must count towards tb->size. So adjust s390x's translate_one() to act this way for both illegal instructions and instructions that are known to generate exceptions. Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- target/s390x/translate.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)