Coverity notes that we forgot to check the error return from
lock_user() in one place in the handling of the UHI_plog semihosting
call.  Add the missing error handling.

report_fault() is rather brutal in that it will call abort(), but
this is the same error-handling used in the rest of this file.

Resolves: Coverity CID 1490684
Fixes: ea4210600db3c5 ("target/mips: Avoid qemu_semihosting_log_out for UHI_plog")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
NB: only tested with 'make check' and 'make check-tcg', which
almost certainly don't actually exercise this codepath.
---
 target/mips/tcg/sysemu/mips-semi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/mips/tcg/sysemu/mips-semi.c b/target/mips/tcg/sysemu/mips-semi.c
index 5fb1ad90920..85f0567a7fa 100644
--- a/target/mips/tcg/sysemu/mips-semi.c
+++ b/target/mips/tcg/sysemu/mips-semi.c
@@ -321,6 +321,9 @@ void mips_semihosting(CPUMIPSState *env)
             if (use_gdb_syscalls()) {
                 addr = gpr[29] - str->len;
                 p = lock_user(VERIFY_WRITE, addr, str->len, 0);
+                if (!p) {
+                    report_fault(env);
+                }
                 memcpy(p, str->str, str->len);
                 unlock_user(p, addr, str->len);
                 semihost_sys_write(cs, uhi_cb, 2, addr, str->len);
-- 
2.25.1

On Tue, Jul 19, 2022 at 9:17 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Coverity notes that we forgot to check the error return from
> lock_user() in one place in the handling of the UHI_plog semihosting
> call.  Add the missing error handling.
>
> report_fault() is rather brutal in that it will call abort(), but
> this is the same error-handling used in the rest of this file.
>
> Resolves: Coverity CID 1490684
> Fixes: ea4210600db3c5 ("target/mips: Avoid qemu_semihosting_log_out for UHI_plog")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> NB: only tested with 'make check' and 'make check-tcg', which
> almost certainly don't actually exercise this codepath.
> ---
>  target/mips/tcg/sysemu/mips-semi.c | 3 +++
>  1 file changed, 3 insertions(+)

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

Queued to mips-fixes, thanks!

On 7/20/22 00:47, Peter Maydell wrote:
> Coverity notes that we forgot to check the error return from
> lock_user() in one place in the handling of the UHI_plog semihosting
> call.  Add the missing error handling.
> 
> report_fault() is rather brutal in that it will call abort(), but
> this is the same error-handling used in the rest of this file.
> 
> Resolves: Coverity CID 1490684
> Fixes: ea4210600db3c5 ("target/mips: Avoid qemu_semihosting_log_out for UHI_plog")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> NB: only tested with 'make check' and 'make check-tcg', which
> almost certainly don't actually exercise this codepath.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

> ---
>   target/mips/tcg/sysemu/mips-semi.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/target/mips/tcg/sysemu/mips-semi.c b/target/mips/tcg/sysemu/mips-semi.c
> index 5fb1ad90920..85f0567a7fa 100644
> --- a/target/mips/tcg/sysemu/mips-semi.c
> +++ b/target/mips/tcg/sysemu/mips-semi.c
> @@ -321,6 +321,9 @@ void mips_semihosting(CPUMIPSState *env)
>               if (use_gdb_syscalls()) {
>                   addr = gpr[29] - str->len;
>                   p = lock_user(VERIFY_WRITE, addr, str->len, 0);
> +                if (!p) {
> +                    report_fault(env);
> +                }
>                   memcpy(p, str->str, str->len);
>                   unlock_user(p, addr, str->len);
>                   semihost_sys_write(cs, uhi_cb, 2, addr, str->len);