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

[David Hildenbrand <david@redhat.com>]

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