diff mbox series

target/s390x: Fix the accumulation of ccm in op_icm

Message ID 20220401193659.332079-1-richard.henderson@linaro.org
State New
Headers show
Series target/s390x: Fix the accumulation of ccm in op_icm | expand

Commit Message

Richard Henderson April 1, 2022, 7:36 p.m. UTC
Coverity rightly reports that 0xff << pos can overflow.
This would affect the ICMH instruction.

Fixes: Coverity CID 1487161
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/tcg/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Thomas Huth April 2, 2022, 8:38 a.m. UTC | #1
On 01/04/2022 21.36, Richard Henderson wrote:
> Coverity rightly reports that 0xff << pos can overflow.
> This would affect the ICMH instruction.
> 
> Fixes: Coverity CID 1487161
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/s390x/tcg/translate.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
> index 5acfc0ff9b..ea7baf0832 100644
> --- a/target/s390x/tcg/translate.c
> +++ b/target/s390x/tcg/translate.c
> @@ -2622,7 +2622,7 @@ static DisasJumpType op_icm(DisasContext *s, DisasOps *o)
>                   tcg_gen_qemu_ld8u(tmp, o->in2, get_mem_index(s));
>                   tcg_gen_addi_i64(o->in2, o->in2, 1);
>                   tcg_gen_deposit_i64(o->out, o->out, tmp, pos, 8);
> -                ccm |= 0xff << pos;
> +                ccm |= 0xffull << pos;
>               }
>               m3 = (m3 << 1) & 0xf;
>               pos -= 8;

Reviewed-by: Thomas Huth <thuth@redhat.com>

Is this still something for 7.0, or can it wait for the 7.1 cycle?
Richard Henderson April 2, 2022, 3:30 p.m. UTC | #2
On 4/2/22 02:38, Thomas Huth wrote:
> On 01/04/2022 21.36, Richard Henderson wrote:
>> Coverity rightly reports that 0xff << pos can overflow.
>> This would affect the ICMH instruction.
>>
>> Fixes: Coverity CID 1487161
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/s390x/tcg/translate.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
>> index 5acfc0ff9b..ea7baf0832 100644
>> --- a/target/s390x/tcg/translate.c
>> +++ b/target/s390x/tcg/translate.c
>> @@ -2622,7 +2622,7 @@ static DisasJumpType op_icm(DisasContext *s, DisasOps *o)
>>                   tcg_gen_qemu_ld8u(tmp, o->in2, get_mem_index(s));
>>                   tcg_gen_addi_i64(o->in2, o->in2, 1);
>>                   tcg_gen_deposit_i64(o->out, o->out, tmp, pos, 8);
>> -                ccm |= 0xff << pos;
>> +                ccm |= 0xffull << pos;
>>               }
>>               m3 = (m3 << 1) & 0xf;
>>               pos -= 8;
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 
> Is this still something for 7.0, or can it wait for the 7.1 cycle?

The bug has been present since 2012, affecting only the cc value of icmh.  It could wait 
for 7.1.

r~
David Hildenbrand April 4, 2022, 8:36 a.m. UTC | #3
On 01.04.22 21:36, Richard Henderson wrote:
> Coverity rightly reports that 0xff << pos can overflow.
> This would affect the ICMH instruction.
> 
> Fixes: Coverity CID 1487161
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/s390x/tcg/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
> index 5acfc0ff9b..ea7baf0832 100644
> --- a/target/s390x/tcg/translate.c
> +++ b/target/s390x/tcg/translate.c
> @@ -2622,7 +2622,7 @@ static DisasJumpType op_icm(DisasContext *s, DisasOps *o)
>                  tcg_gen_qemu_ld8u(tmp, o->in2, get_mem_index(s));
>                  tcg_gen_addi_i64(o->in2, o->in2, 1);
>                  tcg_gen_deposit_i64(o->out, o->out, tmp, pos, 8);
> -                ccm |= 0xff << pos;
> +                ccm |= 0xffull << pos;
>              }
>              m3 = (m3 << 1) & 0xf;
>              pos -= 8;

Reviewed-by: David Hildenbrand <david@redhat.com>
Richard Henderson April 27, 2022, 3 a.m. UTC | #4
On 4/1/22 12:36, Richard Henderson wrote:
> Coverity rightly reports that 0xff << pos can overflow.
> This would affect the ICMH instruction.
> 
> Fixes: Coverity CID 1487161
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/s390x/tcg/translate.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
> index 5acfc0ff9b..ea7baf0832 100644
> --- a/target/s390x/tcg/translate.c
> +++ b/target/s390x/tcg/translate.c
> @@ -2622,7 +2622,7 @@ static DisasJumpType op_icm(DisasContext *s, DisasOps *o)
>                   tcg_gen_qemu_ld8u(tmp, o->in2, get_mem_index(s));
>                   tcg_gen_addi_i64(o->in2, o->in2, 1);
>                   tcg_gen_deposit_i64(o->out, o->out, tmp, pos, 8);
> -                ccm |= 0xff << pos;
> +                ccm |= 0xffull << pos;
>               }
>               m3 = (m3 << 1) & 0xf;
>               pos -= 8;

Queuing to tcg-next.


r~
diff mbox series

Patch

diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 5acfc0ff9b..ea7baf0832 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -2622,7 +2622,7 @@  static DisasJumpType op_icm(DisasContext *s, DisasOps *o)
                 tcg_gen_qemu_ld8u(tmp, o->in2, get_mem_index(s));
                 tcg_gen_addi_i64(o->in2, o->in2, 1);
                 tcg_gen_deposit_i64(o->out, o->out, tmp, pos, 8);
-                ccm |= 0xff << pos;
+                ccm |= 0xffull << pos;
             }
             m3 = (m3 << 1) & 0xf;
             pos -= 8;