:p
atchew
Login
Hi, Christian has reported that one can make a TCG guest stuck by running PRNO TRNG with a huge buffer. This series fixes this by making PRNO TRNG always end after generating 16KB. Patch 1 is the fix, patch 2 is the test. Cc: Harald, because this may conflict with the CPACF series. Best regards, Ilya Ilya Leoshkevich (2): target/s390x: Make PRNO TRNG interruptible tests/tcg/s390x: Test PRNO TRNG interruptibility target/s390x/tcg/crypto_helper.c | 25 +++++++++--- tests/tcg/s390x/Makefile.target | 1 + tests/tcg/s390x/prno-trng.c | 67 ++++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 tests/tcg/s390x/prno-trng.c -- 2.55.0
fill_buf_random() writes the entire guest-requested amount of random bytes in one go. Since the length is a full 64-bit value, a guest can request several gigabytes and keep the vCPU spinning inside the helper, without a chance to react to interrupts. Do the same thing as cpacf_sha512(): process only a limited amount of data per instruction execution and report partial completion with condition code 3. This way the guest reissues the instruction, and pending interrupts can be handled in between. Reported-by: Christian Borntraeger <borntraeger@linux.ibm.com> Fixes: 3dbc5fdacb5a ("target/s390x: support PRNO_TRNG instruction") Cc: qemu-stable@nongnu.org Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- target/s390x/tcg/crypto_helper.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/target/s390x/tcg/crypto_helper.c b/target/s390x/tcg/crypto_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/s390x/tcg/crypto_helper.c +++ b/target/s390x/tcg/crypto_helper.c @@ -XXX,XX +XXX,XX @@ static int cpacf_sha512(CPUS390XState *env, const int mmu_idx, uintptr_t ra, return !len ? 0 : 3; } -static void fill_buf_random(CPUS390XState *env, const int mmu_idx, uintptr_t ra, - uint64_t *buf_reg, uint64_t *len_reg) +static int fill_buf_random(CPUS390XState *env, const int mmu_idx, uintptr_t ra, + uint64_t *buf_reg, uint64_t *len_reg) { + enum { MAX_BYTES_PER_RUN = 8192 }; /* Arbitrary: keep interactivity. */ const MemOpIdx oi = make_memop_idx(MO_8, mmu_idx); uint8_t tmp[256]; - uint64_t len = *len_reg; + uint64_t len = *len_reg, processed = 0; int buf_reg_len = 64; if (!(env->psw.mask & PSW_MASK_64)) { @@ -XXX,XX +XXX,XX @@ static void fill_buf_random(CPUS390XState *env, const int mmu_idx, uintptr_t ra, while (len) { size_t block = MIN(len, sizeof(tmp)); + if (processed >= MAX_BYTES_PER_RUN) { + break; + } + qemu_guest_getrandom_nofail(tmp, block); for (size_t i = 0; i < block; ++i) { cpu_stb_mmu(env, wrap_address(env, *buf_reg), tmp[i], oi, ra); @@ -XXX,XX +XXX,XX @@ static void fill_buf_random(CPUS390XState *env, const int mmu_idx, uintptr_t ra, --*len_reg; } len -= block; + processed += block; } + + return !len ? 0 : 3; } uint32_t HELPER(msa)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t r3, @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(msa)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t r3, uint8_t subfunc[16] = { 0 }; uint64_t param_addr; MemOpIdx oi; + int cc; switch (type) { case S390_FEAT_TYPE_KMAC: @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(msa)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t r3, return cpacf_sha512(env, mmu_idx, ra, env->regs[1], &env->regs[r2], &env->regs[r2 + 1], type); case 114: /* CPACF_PRNO_TRNG */ - fill_buf_random(env, mmu_idx, ra, &env->regs[r1], &env->regs[r1 + 1]); - fill_buf_random(env, mmu_idx, ra, &env->regs[r2], &env->regs[r2 + 1]); - break; + cc = fill_buf_random(env, mmu_idx, ra, + &env->regs[r1], &env->regs[r1 + 1]); + if (cc == 0) { + cc = fill_buf_random(env, mmu_idx, ra, + &env->regs[r2], &env->regs[r2 + 1]); + } + return cc; default: /* we don't implement any other subfunction yet */ g_assert_not_reached(); -- 2.55.0
Add a small test that issues a large PRNO TRNG request while a timer is running, and checks that the timer interrupts it several times. Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- tests/tcg/s390x/Makefile.target | 1 + tests/tcg/s390x/prno-trng.c | 67 +++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 tests/tcg/s390x/prno-trng.c diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target index XXXXXXX..XXXXXXX 100644 --- a/tests/tcg/s390x/Makefile.target +++ b/tests/tcg/s390x/Makefile.target @@ -XXX,XX +XXX,XX @@ Z13_TESTS+=vcksm Z13_TESTS+=vstl Z13_TESTS+=vrep Z13_TESTS+=precise-smc-user +Z13_TESTS+=prno-trng $(Z13_TESTS): CFLAGS+=-march=z13 -O2 TESTS+=$(Z13_TESTS) diff --git a/tests/tcg/s390x/prno-trng.c b/tests/tcg/s390x/prno-trng.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/tests/tcg/s390x/prno-trng.c @@ -XXX,XX +XXX,XX @@ +/* + * Test that PERFORM RANDOM NUMBER OPERATION TRNG is interruptible. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ +#include <assert.h> +#include <signal.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/time.h> +#include <asm/ucontext.h> + +static unsigned char buf1[16 * 1024 * 1024]; +static unsigned char buf2[16 * 1024 * 1024]; + +static volatile sig_atomic_t interrupted; + +static void sigprof_handler(int sig, siginfo_t *info, void *ucontext) +{ + struct ucontext *uc = ucontext; + unsigned long addr = uc->uc_mcontext.regs.psw.addr; + + if (*(unsigned short *)(addr - 4) == 0xb93c) { + interrupted++; + } +} + +static void prno_trng(void *b1, unsigned long l1, void *b2, unsigned long l2) +{ + register unsigned long r0 asm("r0") = 114; /* TRNG */ + register unsigned long r2 asm("r2") = (unsigned long)b1; + register unsigned long r3 asm("r3") = l1; + register unsigned long r4 asm("r4") = (unsigned long)b2; + register unsigned long r5 asm("r5") = l2; + + asm volatile("0: ppno %[r2],%[r4]\n" /* prno alias for old toolchains */ + " jo 0b" + : [r2] "+r" (r2), [r3] "+r" (r3) + , [r4] "+r" (r4), [r5] "+r" (r5) + : "r" (r0) + : "cc", "memory"); +} + +int main(void) +{ + struct itimerval it = { + .it_interval = { .tv_usec = 10000 }, /* 0.01s */ + .it_value = { .tv_usec = 10000 }, + }; + struct sigaction act = { + .sa_sigaction = sigprof_handler, + .sa_flags = SA_SIGINFO, + }; + int err; + + err = sigaction(SIGPROF, &act, NULL); + assert(err == 0); + err = setitimer(ITIMER_PROF, &it, NULL); + assert(err == 0); + + prno_trng(buf1, sizeof(buf1), buf2, sizeof(buf2)); + printf("interrupted %d times\n", interrupted); + assert(interrupted >= 3); + + return EXIT_SUCCESS; +} -- 2.55.0
v1: https://lore.kernel.org/qemu-devel/20260714113946.275122-1-iii@linux.ibm.com/ v1 -> v2: Use cpu_loop_exit_requested() (Richard). Apply R-b, since the change is trivial enough. Hi, Christian has reported that one can make a TCG guest stuck by running PRNO TRNG with a huge buffer. This series fixes this by making PRNO TRNG end on cpu_loop_exit_requested(). Patch 1 is the fix, patch 2 is the test. Cc: Harald, because this may conflict with the CPACF series. Best regards, Ilya Ilya Leoshkevich (2): target/s390x: Make PRNO TRNG interruptible tests/tcg/s390x: Test PRNO TRNG interruptibility target/s390x/tcg/crypto_helper.c | 22 ++++++++--- tests/tcg/s390x/Makefile.target | 1 + tests/tcg/s390x/prno-trng.c | 67 ++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 5 deletions(-) create mode 100644 tests/tcg/s390x/prno-trng.c -- 2.55.0
fill_buf_random() writes the entire guest-requested amount of random bytes in one go. Since the length is a full 64-bit value, a guest can request several gigabytes and keep the vCPU spinning inside the helper, without a chance to react to interrupts. Do the same thing as HELPER(mvcl): check cpu_loop_exit_requested() at the bottom of the loop, and when a return to the main loop is pending, stop and report partial completion with condition code 3. Reported-by: Christian Borntraeger <borntraeger@linux.ibm.com> Fixes: 3dbc5fdacb5a ("target/s390x: support PRNO_TRNG instruction") Cc: qemu-stable@nongnu.org Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- target/s390x/tcg/crypto_helper.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/target/s390x/tcg/crypto_helper.c b/target/s390x/tcg/crypto_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/s390x/tcg/crypto_helper.c +++ b/target/s390x/tcg/crypto_helper.c @@ -XXX,XX +XXX,XX @@ #include "qemu/guest-random.h" #include "s390x-internal.h" #include "tcg_s390x.h" +#include "exec/cpu-common.h" #include "exec/helper-proto.h" #include "accel/tcg/cpu-ldst-common.h" #include "accel/tcg/cpu-mmu-index.h" @@ -XXX,XX +XXX,XX @@ static int cpacf_sha512(CPUS390XState *env, const int mmu_idx, uintptr_t ra, return !len ? 0 : 3; } -static void fill_buf_random(CPUS390XState *env, const int mmu_idx, uintptr_t ra, - uint64_t *buf_reg, uint64_t *len_reg) +static int fill_buf_random(CPUS390XState *env, const int mmu_idx, uintptr_t ra, + uint64_t *buf_reg, uint64_t *len_reg) { const MemOpIdx oi = make_memop_idx(MO_8, mmu_idx); uint8_t tmp[256]; @@ -XXX,XX +XXX,XX @@ static void fill_buf_random(CPUS390XState *env, const int mmu_idx, uintptr_t ra, --*len_reg; } len -= block; + + if (cpu_loop_exit_requested(env_cpu(env))) { + break; + } } + + return len == 0 ? 0 : 3; } uint32_t HELPER(msa)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t r3, @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(msa)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t r3, uint8_t subfunc[16] = { 0 }; uint64_t param_addr; MemOpIdx oi; + int cc; switch (type) { case S390_FEAT_TYPE_KMAC: @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(msa)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t r3, return cpacf_sha512(env, mmu_idx, ra, env->regs[1], &env->regs[r2], &env->regs[r2 + 1], type); case 114: /* CPACF_PRNO_TRNG */ - fill_buf_random(env, mmu_idx, ra, &env->regs[r1], &env->regs[r1 + 1]); - fill_buf_random(env, mmu_idx, ra, &env->regs[r2], &env->regs[r2 + 1]); - break; + cc = fill_buf_random(env, mmu_idx, ra, + &env->regs[r1], &env->regs[r1 + 1]); + if (cc == 0) { + cc = fill_buf_random(env, mmu_idx, ra, + &env->regs[r2], &env->regs[r2 + 1]); + } + return cc; default: /* we don't implement any other subfunction yet */ g_assert_not_reached(); -- 2.55.0
Add a small test that issues a large PRNO TRNG request while a timer is running, and checks that the timer interrupts it several times. Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- tests/tcg/s390x/Makefile.target | 1 + tests/tcg/s390x/prno-trng.c | 67 +++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 tests/tcg/s390x/prno-trng.c diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target index XXXXXXX..XXXXXXX 100644 --- a/tests/tcg/s390x/Makefile.target +++ b/tests/tcg/s390x/Makefile.target @@ -XXX,XX +XXX,XX @@ Z13_TESTS+=vcksm Z13_TESTS+=vstl Z13_TESTS+=vrep Z13_TESTS+=precise-smc-user +Z13_TESTS+=prno-trng $(Z13_TESTS): CFLAGS+=-march=z13 -O2 TESTS+=$(Z13_TESTS) diff --git a/tests/tcg/s390x/prno-trng.c b/tests/tcg/s390x/prno-trng.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/tests/tcg/s390x/prno-trng.c @@ -XXX,XX +XXX,XX @@ +/* + * Test that PERFORM RANDOM NUMBER OPERATION TRNG is interruptible. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ +#include <assert.h> +#include <signal.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/time.h> +#include <asm/ucontext.h> + +static unsigned char buf1[16 * 1024 * 1024]; +static unsigned char buf2[16 * 1024 * 1024]; + +static volatile sig_atomic_t interrupted; + +static void sigprof_handler(int sig, siginfo_t *info, void *ucontext) +{ + struct ucontext *uc = ucontext; + unsigned long addr = uc->uc_mcontext.regs.psw.addr; + + if (*(unsigned short *)(addr - 4) == 0xb93c) { + interrupted++; + } +} + +static void prno_trng(void *b1, unsigned long l1, void *b2, unsigned long l2) +{ + register unsigned long r0 asm("r0") = 114; /* TRNG */ + register unsigned long r2 asm("r2") = (unsigned long)b1; + register unsigned long r3 asm("r3") = l1; + register unsigned long r4 asm("r4") = (unsigned long)b2; + register unsigned long r5 asm("r5") = l2; + + asm volatile("0: ppno %[r2],%[r4]\n" /* prno alias for old toolchains */ + " jo 0b" + : [r2] "+r" (r2), [r3] "+r" (r3) + , [r4] "+r" (r4), [r5] "+r" (r5) + : "r" (r0) + : "cc", "memory"); +} + +int main(void) +{ + struct itimerval it = { + .it_interval = { .tv_usec = 10000 }, /* 0.01s */ + .it_value = { .tv_usec = 10000 }, + }; + struct sigaction act = { + .sa_sigaction = sigprof_handler, + .sa_flags = SA_SIGINFO, + }; + int err; + + err = sigaction(SIGPROF, &act, NULL); + assert(err == 0); + err = setitimer(ITIMER_PROF, &it, NULL); + assert(err == 0); + + prno_trng(buf1, sizeof(buf1), buf2, sizeof(buf2)); + printf("interrupted %d times\n", interrupted); + assert(interrupted >= 3); + + return EXIT_SUCCESS; +} -- 2.55.0