[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH 4/4] semihosting: Fix handling of buffer in TARGET_SYS_TMPNAM
From: |
Peter Maydell |
Subject: |
[PATCH 4/4] semihosting: Fix handling of buffer in TARGET_SYS_TMPNAM |
Date: |
Tue, 19 Jul 2022 13:11:10 +0100 |
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>
---
semihosting/arm-compat-semi.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
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);
--
2.25.1