Peter Maydell <peter.mayd...@linaro.org> writes:
> On Tue, 28 May 2019 at 10:49, Alex Bennée <alex.ben...@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.ben...@linaro.org>
>> Reviewed-by: Richard Henderson <richard.hender...@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().
Mea culpa - I'd avoided the nastiness of the lock string stuff in the
softmmu case but reverted to a naive implementation for linux-user after
the fact. I'll send a fix for that.
> 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.
It is an admittedly in-elegant hack to deal with the fact we call the
same function for outputting a character as well as a string. None of
the guests actually give us the length:
* @len: length of string or 0 (string is null terminated)
We could formalise it by making s/len/is_char/ and making it a bool or
just add some more text to the description.
>
> thanks
> -- PMM
--
Alex Bennée
