Re: [PATCH v4 20/28] pc-bios/s390-ccw: Add signature verification for secure IPL in audit mode

[Collin Walling <walling@linux.ibm.com>]

On 7/14/25 11:34 AM, Thomas Huth wrote:
> On 14/07/2025 16.54, Jared Rossi wrote:
>>
>> [snip...]
>>>> +
>>>> +        entry++;
>>>> +
>>>> +        if ((uint8_t *)(&entry[1]) > tmp_sec + MAX_SECTOR_SIZE) {
>>>> +            puts("Wrong entry value");
>>>> +            return -EINVAL;
>>>> +        }
>>> Can someone who is more informed than I am of the IPL process please
>>> explain to me what is the purpose of the above check?  Why does it check
>>> if the next entry, the one which isn't going to be inspected/loaded, is
>>> within the bounds of tmp_sec?  This has been here since this file's
>>> inception and I can't find any documentation or mention that supports it.
>>>
>>> This code precludes any of the secure IPL changes.
>>>
>>> Was this actually meant to be entry[0] to ensure the actual entry we
>>> want to work on is not outside the bounds of tmp_sec?  Or perhaps it was
>>> meant to be done before the increment to entry?
>>>
>>
>> I noticed that as well and came to the same conclusions as you, which is to 
>> say,
>> it has always been that way and it is not clear what the purpose is, but it 
>> does
>> not appear to have any impact on the proposed secure IPL functionality.

Fair enough.  Let's keep the current code in and address it later.
Thanks, Jared.

> 
> I think it's meant as a check for the *end* of entry[0], so it's likely just 
> a quirky way of saying:
> 
>     if (((uint8_t *)entry) + sizeof(*entry) > tmp_sec + MAX_SECTOR_SIZE)
> 
> ?
> 
>   Thomas
> 

This makes a lot more sense to me.  Thanks, Thomas.

-- 
Regards,
  Collin