Re: [PATCH v12 23/35] pc-bios/s390-ccw: Add signature verification for secure IPL in audit mode

[Jared Rossi <jrossi@linux.ibm.com>]

On 7/1/26 4:49 PM, Zhuoying Cai wrote:
> Enable secure IPL in audit mode, which performs signature verification,
> but any error does not terminate the boot process. Only warnings will be
> logged to the console instead.
>
> Secure IPL in audit mode requires at least one certificate provided in
> the key store along with necessary facilities (Secure IPL Facility,
> Certificate Store Facility and secure IPL extension support).
>
> Note: Secure IPL in audit mode is implemented for the SCSI scheme of
> virtio-blk/virtio-scsi devices.
>
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>   docs/system/s390x/secure-ipl.rst |  15 ++
>   pc-bios/s390-ccw/Makefile        |   2 +-
>   pc-bios/s390-ccw/bootmap.c       |  27 +++
>   pc-bios/s390-ccw/bootmap.h       |   9 +
>   pc-bios/s390-ccw/jump2ipl.c      |   7 +
>   pc-bios/s390-ccw/main.c          |  18 +-
>   pc-bios/s390-ccw/s390-ccw.h      |  20 ++
>   pc-bios/s390-ccw/sclp.c          |  27 +++
>   pc-bios/s390-ccw/sclp.h          |   6 +
>   pc-bios/s390-ccw/secure-ipl.c    | 362 +++++++++++++++++++++++++++++++
>   pc-bios/s390-ccw/secure-ipl.h    | 116 ++++++++++
>   11 files changed, 607 insertions(+), 2 deletions(-)
>   create mode 100644 pc-bios/s390-ccw/secure-ipl.c
>   create mode 100644 pc-bios/s390-ccw/secure-ipl.h
>
[...]
> +
> +void update_cert_list(IplSignatureCertificateList *cert_list)
> +{
> +    IplSignatureCertificateEntry *cert_entry;
> +    uint8_t *cert_buf;
> +
> +    cert_buf = (uint8_t *)qipl.ipl_data;
> +
> +    for_each_rb_entry(cert_entry, cert_list) {
> +        memcpy(cert_buf, (uint8_t *)cert_entry->addr, cert_entry->len);
> +        cert_entry->addr = (uint64_t)cert_buf;
> +        cert_buf += cert_entry->len;
> +    }
> +}

This looks like the correct idea but I think it needs additional 
validation.  Firstly, as Eric mentioned, there is no check that the size 
of our cert entries don't exceed the qipl.ipl_data space.  I know there 
are implicit factors such as maximum certificate size and such that 
limit it in a way that it shouldn't be possible, but we should still check.

Secondly and more critically, you need to reset the qipl.ipl_data 
address to it's initial value before reusing it.  I don't think I saw 
that done anywhere.  The address is incremented after each IPLB is 
loaded from the chain (see the load_next_iplb() function in iplb.h), so 
if we in theory had started with 7 IPLBs in the chain and loaded all of 
them, and if we then copied the maximum allowed size of certificate 
entry data, we would exceed the qipl.ipl_data upper limit and probably 
clobber the kernel.

I think the easiest way to restore the qipl.ipl_data address to its 
initial value is to track how many IPLBs we've used, multiply that by 
the size of the IPLB, and then move the address back by that amount.

Regards,
Jared Rossi