1. Patchew
  2. QEMU
  3. [PATCH 00/14] target/s390x: Miscellaneous TCG fixes, part 2
  4. View patch

[PATCH 07/14] target/s390x: Fix assertion failure in VFMIN/VFMAX with reserved type

Ilya Leoshkevich posted 14 patches 2 years, 6 months ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Thomas Huth <thuth@redhat.com>
There is a newer version of this series
[PATCH 07/14] target/s390x: Fix assertion failure in VFMIN/VFMAX with reserved type
Posted by Ilya Leoshkevich 2 years, 6 months ago 
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;
 
-        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);
-- 
2.41.0
Re: [PATCH 07/14] target/s390x: Fix assertion failure in VFMIN/VFMAX with reserved type
Posted by David Hildenbrand 2 years, 6 months ago 
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);

-- 
Cheers,

David / dhildenb
Re: [PATCH 07/14] target/s390x: Fix assertion failure in VFMIN/VFMAX with reserved type
Posted by Ilya Leoshkevich 2 years, 6 months ago 
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.
Re: [PATCH 07/14] target/s390x: Fix assertion failure in VFMIN/VFMAX with reserved type
Posted by David Hildenbrand 2 years, 6 months ago 
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!

-- 
Cheers,

David / dhildenb

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.