Re: [Qemu-devel] [PULL 06/28] target/arm: use the common interface for WRITE0/WRITEC in arm-semi

[Peter Maydell <peter.maydell@linaro.org>]

On Tue, 28 May 2019 at 10:49, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Now we have a common semihosting console interface use that for our
> string output. However ARM is currently unique in also supporting
> semihosting for linux-user so we need to replicate the API in
> linux-user. If other architectures gain this support we can move the
> file later.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Hi; Coverity points out an issue in this function (CID 1401700):


> +int qemu_semihosting_console_out(CPUArchState *env, target_ulong addr, int len)
> +{
> +    void *s = lock_user_string(addr);
> +    len = write(STDERR_FILENO, s, len ? len : strlen(s));
> +    unlock_user(s, addr, 0);
> +    return len;
> +}

We call lock_user_string(), which can fail and return NULL
if the memory pointed to by addr isn't actually readable.
But we don't check for the error, so we can pass a NULL
pointer to write().

Also it looks a bit dodgy that we are passed in a
specific length value but we then go and look at the length
of the string, but we trust the specific length value over
the length of the string. If len is larger than the real
length of the string (including terminating NUL) then the
write() will read off the end of the string.

thanks
-- PMM