From e19a61eb514dbf7c9a725c7539ce3b6166cd6ac4 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Fri, 31 May 2019 16:33:38 +0200 Subject: [PATCH] s390x/tcg: Store only the necessary amount of doublewords for STFLE The PoP (z14, 7-382) says: Doublewords to the right of the doubleword in which the highest-numbered facility bit is assigned for a model may or may not be stored. However, stack protection in certain binaries can't deal with that. "gzip" example code: f1b4: a7 08 00 03 lhi %r0,3 f1b8: b2 b0 f0 a0 stfle 160(%r15) f1bc: e3 20 f0 b2 00 90 llgc %r2,178(%r15) f1c2: c0 2b 00 00 00 01 nilf %r2,1 f1c8: b2 4f 00 10 ear %r1,%a0 f1cc: b9 14 00 22 lgfr %r2,%r2 f1d0: eb 11 00 20 00 0d sllg %r1,%r1,32 f1d6: b2 4f 00 11 ear %r1,%a1 f1da: d5 07 f0 b8 10 28 clc 184(8,%r15),40(%r1) f1e0: a7 74 00 06 jne f1ec f1e4: eb ef f1 30 00 04 lmg %r14,%r15,304(%r15) f1ea: 07 fe br %r14 f1ec: c0 e5 ff ff 9d 6e brasl %r14,2cc8 <__stack_chk_fail@plt> In QEMU, we currently have: max_bytes = 24 the code asks for (3 + 1) doublewords == 32 bytes. If we write 32 bytes instead of only 24, and return "2 + 1" doublewords ("one less than the number of doulewords needed to contain all of the facility bits"), the example code detects a stack corruption. In my opinion, the code is wrong. However, it seems to work fine on real machines. So let's limit storing to the minimum of the requested and the maximum doublewords. Cc: Stefan Liebler Cc: Andreas Krebbel Reviewed-by: Richard Henderson Signed-off-by: David Hildenbrand --- target/s390x/misc_helper.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c index 34476134a4..10aa617cf9 100644 --- a/target/s390x/misc_helper.c +++ b/target/s390x/misc_helper.c @@ -678,7 +678,13 @@ uint32_t HELPER(stfle)(CPUS390XState *env, uint64_t addr) prepare_stfl(); max_bytes = ROUND_UP(used_stfl_bytes, 8); - for (i = 0; i < count_bytes; ++i) { + + /* + * The PoP says that doublewords beyond the highest-numbered facility + * bit may or may not be stored. However, existing hardware appears to + * not store the words, and existing software depend on that. + */ + for (i = 0; i < MIN(count_bytes, max_bytes); ++i) { cpu_stb_data_ra(env, addr + i, stfl_bytes[i], ra); } -- GitLab