| Message ID | 20230718213531.117976-8-iii@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/s390x: Miscellaneous TCG fixes, part 2 | expand |
On 18.07.23 23:21, Ilya Leoshkevich wrote: > Passing reserved type to VFMIN/VFMAX causes an assertion failure in > vfmin_res() and vfmax_res(). These instructions should raise a > specification exception in this case. > > Cc: qemu-stable@nongnu.org > Fixes: da4807527f3b ("s390x/tcg: Implement VECTOR FP (MAXIMUM|MINIMUM)") > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- > target/s390x/tcg/vec_fpu_helper.c | 24 +++++++++++++++--------- > 1 file changed, 15 insertions(+), 9 deletions(-) > > diff --git a/target/s390x/tcg/vec_fpu_helper.c b/target/s390x/tcg/vec_fpu_helper.c > index 75cf605b9f4..f1671679879 100644 > --- a/target/s390x/tcg/vec_fpu_helper.c > +++ b/target/s390x/tcg/vec_fpu_helper.c > @@ -915,7 +915,7 @@ static void vfminmax32(S390Vector *v1, const S390Vector *v2, > float32 b = s390_vec_read_float32(v3, i); > float32 result; > Why not check for invalid types once first and leave the rest of that function alone? diff --git a/target/s390x/tcg/vec_fpu_helper.c b/target/s390x/tcg/vec_fpu_helper.c index 75cf605b9f..e0b2a78632 100644 --- a/target/s390x/tcg/vec_fpu_helper.c +++ b/target/s390x/tcg/vec_fpu_helper.c @@ -910,6 +910,11 @@ static void vfminmax32(S390Vector *v1, const S390Vector *v2, S390Vector tmp = {}; int i; + if (type > S390_MINMAX_TYPE_F) { + tcg_s390_program_interrupt(env, PGM_SPECIFICATION, retaddr); + } + for (i = 0; i < 4; i++) { float32 a = s390_vec_read_float32(v2, i); float32 b = s390_vec_read_float32(v3, i);
On Wed, 2023-07-19 at 10:40 +0200, David Hildenbrand wrote: > On 18.07.23 23:21, Ilya Leoshkevich wrote: > > Passing reserved type to VFMIN/VFMAX causes an assertion failure in > > vfmin_res() and vfmax_res(). These instructions should raise a > > specification exception in this case. > > > > Cc: qemu-stable@nongnu.org > > Fixes: da4807527f3b ("s390x/tcg: Implement VECTOR FP > > (MAXIMUM|MINIMUM)") > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > > --- > > target/s390x/tcg/vec_fpu_helper.c | 24 +++++++++++++++--------- > > 1 file changed, 15 insertions(+), 9 deletions(-) > > > > diff --git a/target/s390x/tcg/vec_fpu_helper.c > > b/target/s390x/tcg/vec_fpu_helper.c > > index 75cf605b9f4..f1671679879 100644 > > --- a/target/s390x/tcg/vec_fpu_helper.c > > +++ b/target/s390x/tcg/vec_fpu_helper.c > > @@ -915,7 +915,7 @@ static void vfminmax32(S390Vector *v1, const > > S390Vector *v2, > > float32 b = s390_vec_read_float32(v3, i); > > float32 result; > > > > Why not check for invalid types once first and leave the rest of that > function alone? > > diff --git a/target/s390x/tcg/vec_fpu_helper.c > b/target/s390x/tcg/vec_fpu_helper.c > index 75cf605b9f..e0b2a78632 100644 > --- a/target/s390x/tcg/vec_fpu_helper.c > +++ b/target/s390x/tcg/vec_fpu_helper.c > @@ -910,6 +910,11 @@ static void vfminmax32(S390Vector *v1, const > S390Vector *v2, > S390Vector tmp = {}; > int i; > > + if (type > S390_MINMAX_TYPE_F) { > + tcg_s390_program_interrupt(env, PGM_SPECIFICATION, retaddr); > + } > + > for (i = 0; i < 4; i++) { > float32 a = s390_vec_read_float32(v2, i); > float32 b = s390_vec_read_float32(v3, i); > I have taken another look, and turns out there already is: static DisasJumpType op_vfmax(DisasContext *s, DisasOps *o) { ... if (m6 == 5 || m6 == 6 || m6 == 7 || m6 > 13) { gen_program_exception(s, PGM_SPECIFICATION); return DISAS_NORETURN; } What the fuzzer has found was the m6 == 13 case, so only a small adjustment is needed.
On 19.07.23 11:34, Ilya Leoshkevich wrote: > On Wed, 2023-07-19 at 10:40 +0200, David Hildenbrand wrote: >> On 18.07.23 23:21, Ilya Leoshkevich wrote: >>> Passing reserved type to VFMIN/VFMAX causes an assertion failure in >>> vfmin_res() and vfmax_res(). These instructions should raise a >>> specification exception in this case. >>> >>> Cc: qemu-stable@nongnu.org >>> Fixes: da4807527f3b ("s390x/tcg: Implement VECTOR FP >>> (MAXIMUM|MINIMUM)") >>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> >>> --- >>> target/s390x/tcg/vec_fpu_helper.c | 24 +++++++++++++++--------- >>> 1 file changed, 15 insertions(+), 9 deletions(-) >>> >>> diff --git a/target/s390x/tcg/vec_fpu_helper.c >>> b/target/s390x/tcg/vec_fpu_helper.c >>> index 75cf605b9f4..f1671679879 100644 >>> --- a/target/s390x/tcg/vec_fpu_helper.c >>> +++ b/target/s390x/tcg/vec_fpu_helper.c >>> @@ -915,7 +915,7 @@ static void vfminmax32(S390Vector *v1, const >>> S390Vector *v2, >>> float32 b = s390_vec_read_float32(v3, i); >>> float32 result; >>> >> >> Why not check for invalid types once first and leave the rest of that >> function alone? >> >> diff --git a/target/s390x/tcg/vec_fpu_helper.c >> b/target/s390x/tcg/vec_fpu_helper.c >> index 75cf605b9f..e0b2a78632 100644 >> --- a/target/s390x/tcg/vec_fpu_helper.c >> +++ b/target/s390x/tcg/vec_fpu_helper.c >> @@ -910,6 +910,11 @@ static void vfminmax32(S390Vector *v1, const >> S390Vector *v2, >> S390Vector tmp = {}; >> int i; >> >> + if (type > S390_MINMAX_TYPE_F) { >> + tcg_s390_program_interrupt(env, PGM_SPECIFICATION, retaddr); >> + } >> + >> for (i = 0; i < 4; i++) { >> float32 a = s390_vec_read_float32(v2, i); >> float32 b = s390_vec_read_float32(v3, i); >> > > I have taken another look, and turns out there already is: > > static DisasJumpType op_vfmax(DisasContext *s, DisasOps *o) > { > ... > > if (m6 == 5 || m6 == 6 || m6 == 7 || m6 > 13) { > gen_program_exception(s, PGM_SPECIFICATION); > return DISAS_NORETURN; > } > > What the fuzzer has found was the m6 == 13 case, so only a small > adjustment is needed. > Oh, good!
diff --git a/target/s390x/tcg/vec_fpu_helper.c b/target/s390x/tcg/vec_fpu_helper.c index 75cf605b9f4..f1671679879 100644 --- a/target/s390x/tcg/vec_fpu_helper.c +++ b/target/s390x/tcg/vec_fpu_helper.c @@ -915,7 +915,7 @@ static void vfminmax32(S390Vector *v1, const S390Vector *v2, float32 b = s390_vec_read_float32(v3, i); float32 result; - if (type != S390_MINMAX_TYPE_IEEE) { + if (type > S390_MINMAX_TYPE_IEEE && type <= S390_MINMAX_TYPE_F) { S390MinMaxRes res; if (is_abs) { @@ -944,12 +944,14 @@ static void vfminmax32(S390Vector *v1, const S390Vector *v2, default: g_assert_not_reached(); } - } else if (!is_abs) { + } else if (type == S390_MINMAX_TYPE_IEEE && !is_abs) { result = is_min ? float32_minnum(a, b, &env->fpu_status) : float32_maxnum(a, b, &env->fpu_status); - } else { + } else if (type == S390_MINMAX_TYPE_IEEE) { result = is_min ? float32_minnummag(a, b, &env->fpu_status) : float32_maxnummag(a, b, &env->fpu_status); + } else { + tcg_s390_program_interrupt(env, PGM_SPECIFICATION, retaddr); } s390_vec_write_float32(&tmp, i, result); @@ -977,7 +979,7 @@ static void vfminmax64(S390Vector *v1, const S390Vector *v2, float64 b = s390_vec_read_float64(v3, i); float64 result; - if (type != S390_MINMAX_TYPE_IEEE) { + if (type > S390_MINMAX_TYPE_IEEE && type <= S390_MINMAX_TYPE_F) { S390MinMaxRes res; if (is_abs) { @@ -1006,12 +1008,14 @@ static void vfminmax64(S390Vector *v1, const S390Vector *v2, default: g_assert_not_reached(); } - } else if (!is_abs) { + } else if (type == S390_MINMAX_TYPE_IEEE && !is_abs) { result = is_min ? float64_minnum(a, b, &env->fpu_status) : float64_maxnum(a, b, &env->fpu_status); - } else { + } else if (type == S390_MINMAX_TYPE_IEEE) { result = is_min ? float64_minnummag(a, b, &env->fpu_status) : float64_maxnummag(a, b, &env->fpu_status); + } else { + tcg_s390_program_interrupt(env, PGM_SPECIFICATION, retaddr); } s390_vec_write_float64(&tmp, i, result); @@ -1035,7 +1039,7 @@ static void vfminmax128(S390Vector *v1, const S390Vector *v2, uint8_t vxc, vec_exc = 0; float128 result; - if (type != S390_MINMAX_TYPE_IEEE) { + if (type > S390_MINMAX_TYPE_IEEE && type <= S390_MINMAX_TYPE_F) { S390MinMaxRes res; if (is_abs) { @@ -1064,12 +1068,14 @@ static void vfminmax128(S390Vector *v1, const S390Vector *v2, default: g_assert_not_reached(); } - } else if (!is_abs) { + } else if (type == S390_MINMAX_TYPE_IEEE && !is_abs) { result = is_min ? float128_minnum(a, b, &env->fpu_status) : float128_maxnum(a, b, &env->fpu_status); - } else { + } else if (type == S390_MINMAX_TYPE_IEEE) { result = is_min ? float128_minnummag(a, b, &env->fpu_status) : float128_maxnummag(a, b, &env->fpu_status); + } else { + tcg_s390_program_interrupt(env, PGM_SPECIFICATION, retaddr); } vxc = check_ieee_exc(env, 0, false, &vec_exc);
Passing reserved type to VFMIN/VFMAX causes an assertion failure in vfmin_res() and vfmax_res(). These instructions should raise a specification exception in this case. Cc: qemu-stable@nongnu.org Fixes: da4807527f3b ("s390x/tcg: Implement VECTOR FP (MAXIMUM|MINIMUM)") Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- target/s390x/tcg/vec_fpu_helper.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-)