Message ID | 20220401193659.332079-1-richard.henderson@linaro.org |
---|---|
State | Superseded |
Headers | show |
Series | target/s390x: Fix the accumulation of ccm in op_icm | expand |
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?
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~
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>
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 --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;
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(-)