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 <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20220719121110.225657-5-peter.maydell@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Message-Id: <20220725140520.515340-10-alex.bennee@linaro.org>
This commit is contained in:
parent
fed49cdf6a
commit
9b1268f55c
@ -504,16 +504,25 @@ void do_common_semihosting(CPUState *cs)
|
|||||||
GET_ARG(1);
|
GET_ARG(1);
|
||||||
GET_ARG(2);
|
GET_ARG(2);
|
||||||
len = asprintf(&s, "/tmp/qemu-%x%02x", getpid(), (int)arg1 & 0xff);
|
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 */
|
/* 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);
|
common_semi_set_ret(cs, -1);
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
p = lock_user(VERIFY_WRITE, arg0, len, 0);
|
p = lock_user(VERIFY_WRITE, arg0, len, 0);
|
||||||
if (!p) {
|
if (!p) {
|
||||||
|
free(s);
|
||||||
goto do_fault;
|
goto do_fault;
|
||||||
}
|
}
|
||||||
memcpy(p, s, len + 1);
|
memcpy(p, s, len);
|
||||||
unlock_user(p, arg0, len);
|
unlock_user(p, arg0, len);
|
||||||
free(s);
|
free(s);
|
||||||
common_semi_set_ret(cs, 0);
|
common_semi_set_ret(cs, 0);
|
||||||
|
Loading…
x
Reference in New Issue
Block a user