| Message ID | 20220719191737.384744-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/mips: Handle lock_user() failure in UHI_plog semihosting call | expand |
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);
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!
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);
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(+)