When decoding a duplex instruction, if the slot0 sub-instruction fails to decode after slot1 succeeds, QEMU was leaving the packet in a partially-decoded state. This allowed invalid duplex encodings (where one sub-instruction doesn't match any valid pattern) to be executed incorrectly.
Fix by resetting the decoder state when slot0 fails, returning an empty instruction that triggers an exception. Add gen_exception_decode_fail() for raising exceptions when decode fails before ctx->next_PC is initialized. This keeps gen_exception_end_tb() semantics unchanged (it continues to use ctx->next_PC for the exception PC after successful decode). Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3291 Signed-off-by: Brian Cain <[email protected]> Reviewed-by: Pierrick Bouvier <[email protected]> --- linux-user/hexagon/cpu_loop.c | 4 ++ target/hexagon/decode.c | 13 ++++- target/hexagon/translate.c | 18 ++++++- tests/tcg/hexagon/invalid-encoding.c | 81 ++++++++++++++++++++++++++++ tests/tcg/hexagon/Makefile.target | 1 + 5 files changed, 113 insertions(+), 4 deletions(-) create mode 100644 tests/tcg/hexagon/invalid-encoding.c diff --git a/linux-user/hexagon/cpu_loop.c b/linux-user/hexagon/cpu_loop.c index 1941f4c9c1..c0e1098e3f 100644 --- a/linux-user/hexagon/cpu_loop.c +++ b/linux-user/hexagon/cpu_loop.c @@ -64,6 +64,10 @@ void cpu_loop(CPUHexagonState *env) force_sig_fault(TARGET_SIGBUS, TARGET_BUS_ADRALN, env->gpr[HEX_REG_R31]); break; + case HEX_CAUSE_INVALID_PACKET: + force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLOPC, + env->gpr[HEX_REG_PC]); + break; case EXCP_ATOMIC: cpu_exec_step_atomic(cs); break; diff --git a/target/hexagon/decode.c b/target/hexagon/decode.c index b5ece60450..69ba1ec96c 100644 --- a/target/hexagon/decode.c +++ b/target/hexagon/decode.c @@ -509,8 +509,14 @@ decode_insns(DisasContext *ctx, Insn *insn, uint32_t encoding) insn->iclass = iclass_bits(encoding); return 2; } + /* + * Slot0 decode failed after slot1 succeeded. This is an invalid + * duplex encoding (both sub-instructions must be valid). + */ + ctx->insn = --insn; } - g_assert_not_reached(); + /* Invalid duplex encoding - return 0 to signal failure */ + return 0; } } @@ -674,7 +680,10 @@ int decode_packet(DisasContext *ctx, int max_words, const uint32_t *words, encoding32 = words[words_read]; end_of_packet = is_packet_end(encoding32); new_insns = decode_insns(ctx, insn, encoding32); - g_assert(new_insns > 0); + if (new_insns == 0) { + /* Invalid instruction encoding */ + return 0; + } /* * If we saw an extender, mark next word extended so immediate * decode works diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c index e88e19cc1a..7fe8b35351 100644 --- a/target/hexagon/translate.c +++ b/target/hexagon/translate.c @@ -195,7 +195,21 @@ static void gen_exception_end_tb(DisasContext *ctx, int excp) tcg_gen_movi_tl(hex_gpr[HEX_REG_PC], ctx->next_PC); gen_exception_raw(excp); ctx->base.is_jmp = DISAS_NORETURN; +} +/* + * Generate exception for decode failures. Unlike gen_exception_end_tb, + * this is used when decode fails before ctx->next_PC is initialized. + */ +static void gen_exception_decode_fail(DisasContext *ctx, int nwords, int excp) +{ + target_ulong fail_pc = ctx->base.pc_next + nwords * sizeof(uint32_t); + + gen_exec_counters(ctx); + tcg_gen_movi_tl(hex_gpr[HEX_REG_PC], fail_pc); + gen_exception_raw(excp); + ctx->base.is_jmp = DISAS_NORETURN; + ctx->base.pc_next = fail_pc; } static int read_packet_words(CPUHexagonState *env, DisasContext *ctx, @@ -935,7 +949,7 @@ static void decode_and_translate_packet(CPUHexagonState *env, DisasContext *ctx) nwords = read_packet_words(env, ctx, words); if (!nwords) { - gen_exception_end_tb(ctx, HEX_CAUSE_INVALID_PACKET); + gen_exception_decode_fail(ctx, 0, HEX_CAUSE_INVALID_PACKET); return; } @@ -950,7 +964,7 @@ static void decode_and_translate_packet(CPUHexagonState *env, DisasContext *ctx) gen_commit_packet(ctx); ctx->base.pc_next += pkt.encod_pkt_size_in_bytes; } else { - gen_exception_end_tb(ctx, HEX_CAUSE_INVALID_PACKET); + gen_exception_decode_fail(ctx, nwords, HEX_CAUSE_INVALID_PACKET); } } diff --git a/tests/tcg/hexagon/invalid-encoding.c b/tests/tcg/hexagon/invalid-encoding.c new file mode 100644 index 0000000000..010a5eb741 --- /dev/null +++ b/tests/tcg/hexagon/invalid-encoding.c @@ -0,0 +1,81 @@ +/* + * Test that invalid instruction encodings are properly rejected. + * + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include <assert.h> +#include <signal.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> + +static void *resume_pc; + +static void handle_sigill(int sig, siginfo_t *info, void *puc) +{ + ucontext_t *uc = (ucontext_t *)puc; + + if (sig != SIGILL) { + _exit(EXIT_FAILURE); + } + + uc->uc_mcontext.r0 = SIGILL; + uc->uc_mcontext.pc = (unsigned long)resume_pc; +} + +/* + * Each test function: + * - Sets r0 to something other than SIGILL + * - Stores the resume address into resume_pc + * - Executes the invalid encoding + * - The handler sets r0 = SIGILL and resumes after the faulting packet + * - Returns the value in r0 + */ + +/* + * Invalid duplex encoding (issue #3291): + * - Word 0: 0x0fff6fff = immext(#0xfffbffc0), parse bits = 01 + * - Word 1: 0x600237b0 = duplex with: + * - slot0 = 0x17b0 (invalid S2 subinstruction encoding) + * - slot1 = 0x0002 (valid SA1_addi) + * - duplex iclass = 7 (S2 for slot0, A for slot1) + * + * Since slot0 doesn't decode to any valid S2 subinstruction, this packet + * should be rejected and raise SIGILL. + */ +static int test_invalid_duplex(void) +{ + int sig; + + asm volatile( + "r0 = #0\n" + "r1 = ##1f\n" + "memw(%1) = r1\n" + ".word 0x0fff6fff\n" /* immext(#0xfffbffc0), parse=01 */ + ".word 0x600237b0\n" /* duplex: slot0=0x17b0 (invalid) */ + "1:\n" + "%0 = r0\n" + : "=r"(sig) + : "r"(&resume_pc) + : "r0", "r1", "memory"); + + return sig; +} + +int main() +{ + struct sigaction act; + + memset(&act, 0, sizeof(act)); + act.sa_sigaction = handle_sigill; + act.sa_flags = SA_SIGINFO; + assert(sigaction(SIGILL, &act, NULL) == 0); + + assert(test_invalid_duplex() == SIGILL); + + puts("PASS"); + return EXIT_SUCCESS; +} diff --git a/tests/tcg/hexagon/Makefile.target b/tests/tcg/hexagon/Makefile.target index e5182c01d8..16669e04a8 100644 --- a/tests/tcg/hexagon/Makefile.target +++ b/tests/tcg/hexagon/Makefile.target @@ -51,6 +51,7 @@ HEX_TESTS += scatter_gather HEX_TESTS += hvx_misc HEX_TESTS += hvx_histogram HEX_TESTS += invalid-slots +HEX_TESTS += invalid-encoding HEX_TESTS += unaligned_pc run-and-check-exception = $(call run-test,$2,$3 2>$2.stderr; \ -- 2.34.1
