diff mbox series

[v1,06/13] semihosting: Don't return negative values on qemu_semihosting_console_write() failure

Message ID 20220725140520.515340-7-alex.bennee@linaro.org
State Superseded
Headers show
Series fixes for 7.1 (testing, docs, semihosting) | expand

Commit Message

Alex Bennée July 25, 2022, 2:05 p.m. UTC
From: Peter Maydell <peter.maydell@linaro.org>

The documentation comment for qemu_semihosting_console_write() says
 * Returns: number of bytes written -- this should only ever be short
 * on some sort of i/o error.

and the callsites rely on this.  However, the implementation code
path which sends console output to a chardev doesn't honour this,
and will return negative values on error.  Bring it into line with
the other implementation codepaths and the documentation, so that
it returns 0 on error.

Spotted by Coverity, because console_write() passes the return value
to unlock_user(), which doesn't accept a negative length.

Resolves: Coverity CID 1490288
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20220719121110.225657-2-peter.maydell@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 semihosting/console.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Philippe Mathieu-Daudé July 26, 2022, 2:31 p.m. UTC | #1
On 25/7/22 16:05, Alex Bennée wrote:
> From: Peter Maydell <peter.maydell@linaro.org>
> 
> The documentation comment for qemu_semihosting_console_write() says
>   * Returns: number of bytes written -- this should only ever be short
>   * on some sort of i/o error.
> 
> and the callsites rely on this.  However, the implementation code
> path which sends console output to a chardev doesn't honour this,
> and will return negative values on error.  Bring it into line with
> the other implementation codepaths and the documentation, so that
> it returns 0 on error.
> 
> Spotted by Coverity, because console_write() passes the return value
> to unlock_user(), which doesn't accept a negative length.
> 
> Resolves: Coverity CID 1490288
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Message-Id: <20220719121110.225657-2-peter.maydell@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   semihosting/console.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
diff mbox series

Patch

diff --git a/semihosting/console.c b/semihosting/console.c
index 5b1ec0a1c3..0f976fe8cb 100644
--- a/semihosting/console.c
+++ b/semihosting/console.c
@@ -111,7 +111,8 @@  int qemu_semihosting_console_read(CPUState *cs, void *buf, int len)
 int qemu_semihosting_console_write(void *buf, int len)
 {
     if (console.chr) {
-        return qemu_chr_write_all(console.chr, (uint8_t *)buf, len);
+        int r = qemu_chr_write_all(console.chr, (uint8_t *)buf, len);
+        return r < 0 ? 0 : r;
     } else {
         return fwrite(buf, 1, len, stderr);
     }