From: Peter Maydell Date: Mon, 25 Jul 2022 14:05:16 +0000 (+0100) Subject: semihosting: Fix handling of buffer in TARGET_SYS_TMPNAM X-Git-Tag: v7.1.0-rc1~14^2~4 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9b1268f55ceb0d9390a051cad299b3021dfa9896;p=thirdparty%2Fqemu.git semihosting: Fix handling of buffer in TARGET_SYS_TMPNAM The TARGET_SYS_TMPNAM implementation has two bugs spotted by Coverity: * confusion about whether 'len' has the length of the string including or excluding the terminating NUL means we lock_user() len bytes of memory but memcpy() len + 1 bytes * In the error-exit cases we forget to free() the buffer that asprintf() returned to us Resolves: Coverity CID 1490285, 1490289 Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-Id: <20220719121110.225657-5-peter.maydell@linaro.org> Signed-off-by: Alex Bennée Message-Id: <20220725140520.515340-10-alex.bennee@linaro.org> --- diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c index d12288fc806..e741674238f 100644 --- a/semihosting/arm-compat-semi.c +++ b/semihosting/arm-compat-semi.c @@ -504,16 +504,25 @@ void do_common_semihosting(CPUState *cs) GET_ARG(1); GET_ARG(2); len = asprintf(&s, "/tmp/qemu-%x%02x", getpid(), (int)arg1 & 0xff); + if (len < 0) { + common_semi_set_ret(cs, -1); + break; + } + + /* Allow for trailing NUL */ + len++; /* Make sure there's enough space in the buffer */ - if (len < 0 || len >= arg2) { + if (len > arg2) { + free(s); common_semi_set_ret(cs, -1); break; } p = lock_user(VERIFY_WRITE, arg0, len, 0); if (!p) { + free(s); goto do_fault; } - memcpy(p, s, len + 1); + memcpy(p, s, len); unlock_user(p, arg0, len); free(s); common_semi_set_ret(cs, 0);