Re: [Qemu-devel] [PATCH for-2.9 3/6] disas/m68k: Avoid unintended sign extension in get_field()

[Laurent Vivier]

Le 03/03/2017 à 16:50, Peter Maydell a écrit :
> In get_field(), we take an 'unsigned char' value and shift it left,
> which implicitly promotes it to 'signed int', before ORing it into an
> 'unsigned long' type.  If 'unsigned long' is 64 bits then this will
> result in a sign extension and the top 32 bits of the result will be
> 1s.  Add explicit casts to unsigned long before shifting to prevent
> this.
> 
> (Spotted by Coverity, CID 715697.)
> 
> Signed-off-by: Peter Maydell <address@hidden>

Reviewed-by: Laurent Vivier <address@hidden>

> ---
>  disas/m68k.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/disas/m68k.c b/disas/m68k.c
> index 073abb9..61b689e 100644
> --- a/disas/m68k.c
> +++ b/disas/m68k.c
> @@ -4685,10 +4685,11 @@ get_field (const unsigned char *data, enum 
> floatformat_byteorders order,
>       /* This is the last byte; zero out the bits which are not part of
>          this field.  */
>       result |=
> -       (*(data + cur_byte) & ((1 << (len - cur_bitshift)) - 1))
> +       (unsigned long)(*(data + cur_byte)
> +                       & ((1 << (len - cur_bitshift)) - 1))
>           << cur_bitshift;
>        else
> -     result |= *(data + cur_byte) << cur_bitshift;
> +     result |= (unsigned long)*(data + cur_byte) << cur_bitshift;
>        cur_bitshift += FLOATFORMAT_CHAR_BIT;
>        if (order == floatformat_little)
>       ++cur_byte;
>