Re: [PATCH v4 31/53] semihosting: Bound length for semihost_sys_{read, write}

[Luc Michel]

On 13:45 Tue 07 Jun     , Richard Henderson wrote:
> Fixes a minor bug in which a 64-bit guest on a 32-bit host could
> truncate the length.  This would only ever cause a problem if
> there were no bits set in the low 32, so that it truncates to 0.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Luc Michel <lmichel@kalray.eu>

> ---
>  semihosting/syscalls.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/semihosting/syscalls.c b/semihosting/syscalls.c
> index 5cb12d6adc..eefbae74f1 100644
> --- a/semihosting/syscalls.c
> +++ b/semihosting/syscalls.c
> @@ -283,6 +283,14 @@ void semihost_sys_close(CPUState *cs, 
> gdb_syscall_complete_cb complete, int fd)
>  void semihost_sys_read_gf(CPUState *cs, gdb_syscall_complete_cb complete,
>                            GuestFD *gf, target_ulong buf, target_ulong len)
>  {
> +    /*
> +     * Bound length for 64-bit guests on 32-bit hosts, not overlowing 
> ssize_t.
> +     * Note the Linux kernel does this with MAX_RW_COUNT, so it's not a bad
> +     * idea to do this unconditionally.
> +     */
> +    if (len > INT32_MAX) {
> +        len = INT32_MAX;
> +    }
>      switch (gf->type) {
>      case GuestFDGDB:
>          gdb_read(cs, complete, gf, buf, len);
> @@ -313,6 +321,14 @@ void semihost_sys_read(CPUState *cs, 
> gdb_syscall_complete_cb complete,
>  void semihost_sys_write_gf(CPUState *cs, gdb_syscall_complete_cb complete,
>                             GuestFD *gf, target_ulong buf, target_ulong len)
>  {
> +    /*
> +     * Bound length for 64-bit guests on 32-bit hosts, not overlowing 
> ssize_t.
> +     * Note the Linux kernel does this with MAX_RW_COUNT, so it's not a bad
> +     * idea to do this unconditionally.
> +     */
> +    if (len > INT32_MAX) {
> +        len = INT32_MAX;
> +    }
>      switch (gf->type) {
>      case GuestFDGDB:
>          gdb_write(cs, complete, gf, buf, len);
> -- 
> 2.34.1
> 
> 
> 
> 
> To declare a filtering error, please use the following link : 
> https://www.security-mail.net/reporter.php?mid=17321.629fcd3b.c73a0.0&r=lmichel%40kalrayinc.com&s=qemu-devel-bounces%2Blmichel%3Dkalrayinc.com%40nongnu.org&o=%5BPATCH+v4+31%2F53%5D+semihosting%3A+Bound+length+for+semihost_sys_%7Bread%2C+write%7D&verdict=C&c=7f9ee3dfa94cc38c566bd57258d2a28de21afd3e
> 

--