[PATCH v8 20/30] pc-bios/s390-ccw: Add signed component address overlap checks

Zhuoying Cai posted 30 patches 1 month, 4 weeks ago
Maintainers: "Daniel P. Berrangé" <berrange@redhat.com>, Pierrick Bouvier <pierrick.bouvier@linaro.org>, Thomas Huth <thuth@redhat.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Richard Henderson <richard.henderson@linaro.org>, Ilya Leoshkevich <iii@linux.ibm.com>, David Hildenbrand <david@kernel.org>, Jared Rossi <jrossi@linux.ibm.com>, Zhuoying Cai <zycai@linux.ibm.com>, Jason Herne <jjherne@linux.ibm.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Hendrik Brueckner <brueckner@linux.ibm.com>
There is a newer version of this series
[PATCH v8 20/30] pc-bios/s390-ccw: Add signed component address overlap checks
Posted by Zhuoying Cai 1 month, 4 weeks ago
Add address range tracking and overlap checks to ensure that no
component overlaps with a signed component during secure IPL.

Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
---
 pc-bios/s390-ccw/secure-ipl.c | 54 +++++++++++++++++++++++++++++++++++
 pc-bios/s390-ccw/secure-ipl.h |  6 ++++
 2 files changed, 60 insertions(+)

diff --git a/pc-bios/s390-ccw/secure-ipl.c b/pc-bios/s390-ccw/secure-ipl.c
index 1d5c2d40ce..27d2833642 100644
--- a/pc-bios/s390-ccw/secure-ipl.c
+++ b/pc-bios/s390-ccw/secure-ipl.c
@@ -210,6 +210,55 @@ static void init_lists(IplDeviceComponentList *comp_list,
     cert_list->ipl_info_header.len = sizeof(cert_list->ipl_info_header);
 }
 
+static bool is_comp_overlap(SecureIplCompAddrRange *comp_addr_range,
+                            int addr_range_index,
+                            uint64_t start_addr, uint64_t end_addr)
+{
+    /* neither a signed nor an unsigned component can overlap with a signed component */
+    for (int i = 0; i < addr_range_index; i++) {
+        if ((comp_addr_range[i].start_addr < end_addr &&
+            start_addr < comp_addr_range[i].end_addr) &&
+            comp_addr_range[i].is_signed) {
+            return true;
+       }
+    }
+
+    return false;
+}
+
+static void comp_addr_range_add(SecureIplCompAddrRange *comp_addr_range,
+                                int addr_range_index, bool is_signed,
+                                uint64_t start_addr, uint64_t end_addr)
+{
+    if (addr_range_index >= MAX_CERTIFICATES) {
+        printf("Warning: Ignoring component address range index [%d]"
+               " because the maximum supported index is %d\n",
+               addr_range_index, MAX_CERTIFICATES - 1);
+        return;
+    }
+
+    comp_addr_range[addr_range_index].is_signed = is_signed;
+    comp_addr_range[addr_range_index].start_addr = start_addr;
+    comp_addr_range[addr_range_index].end_addr = end_addr;
+}
+
+static void addr_overlap_check(SecureIplCompAddrRange *comp_addr_range,
+                               int *addr_range_index,
+                               uint64_t start_addr, uint64_t end_addr, bool is_signed)
+{
+    bool overlap;
+
+    overlap = is_comp_overlap(comp_addr_range, *addr_range_index,
+                              start_addr, end_addr);
+    if (overlap) {
+        zipl_secure_handle("Component addresses overlap");
+    }
+
+    comp_addr_range_add(comp_addr_range, *addr_range_index, is_signed,
+                        start_addr, end_addr);
+    *addr_range_index += 1;
+}
+
 static int zipl_load_signature(ComponentEntry *entry, uint64_t sig_sec)
 {
     if (zipl_load_segment(entry, sig_sec) < 0) {
@@ -253,6 +302,8 @@ int zipl_run_secure(ComponentEntry **entry_ptr, uint8_t *tmp_sec)
      * exists for the certificate).
      */
     int cert_list_table[MAX_CERTIFICATES] = { [0 ... MAX_CERTIFICATES - 1] = -1 };
+    SecureIplCompAddrRange comp_addr_range[MAX_CERTIFICATES];
+    int addr_range_index = 0;
     int signed_count = 0;
 
     if (!secure_ipl_supported()) {
@@ -282,6 +333,9 @@ int zipl_run_secure(ComponentEntry **entry_ptr, uint8_t *tmp_sec)
                 goto out;
             }
 
+            addr_overlap_check(comp_addr_range, &addr_range_index,
+                               comp_addr, comp_addr + comp_len, sig_len > 0);
+
             if (!sig_len) {
                 break;
             }
diff --git a/pc-bios/s390-ccw/secure-ipl.h b/pc-bios/s390-ccw/secure-ipl.h
index eb5ba0ed47..69edfce241 100644
--- a/pc-bios/s390-ccw/secure-ipl.h
+++ b/pc-bios/s390-ccw/secure-ipl.h
@@ -16,6 +16,12 @@
 VCStorageSizeBlock *zipl_secure_get_vcssb(void);
 int zipl_run_secure(ComponentEntry **entry_ptr, uint8_t *tmp_sec);
 
+typedef struct SecureIplCompAddrRange {
+    bool is_signed;
+    uint64_t start_addr;
+    uint64_t end_addr;
+} SecureIplCompAddrRange;
+
 static inline void zipl_secure_handle(const char *message)
 {
     switch (boot_mode) {
-- 
2.52.0
Re: [PATCH v8 20/30] pc-bios/s390-ccw: Add signed component address overlap checks
Posted by Farhan Ali 1 month, 1 week ago
On 2/12/2026 12:43 PM, Zhuoying Cai wrote:
> +static bool is_comp_overlap(SecureIplCompAddrRange *comp_addr_range,
> +                            int addr_range_index,
> +                            uint64_t start_addr, uint64_t end_addr)
> +{
> +    /* neither a signed nor an unsigned component can overlap with a signed component */
> +    for (int i = 0; i < addr_range_index; i++) {
> +        if ((comp_addr_range[i].start_addr < end_addr &&
> +            start_addr < comp_addr_range[i].end_addr) &&
> +            comp_addr_range[i].is_signed) {
> +            return true;
> +       }
> +    }
> +
> +    return false;
> +}
> +

Shouldn't we use <= and >= checks? For example if 
comp_addr_range[i].start_addr == end_addr, wouldn't that be an overlap? 
and similar for start_addr?

Thanks

Farhan
Re: [PATCH v8 20/30] pc-bios/s390-ccw: Add signed component address overlap checks
Posted by Zhuoying Cai 1 month, 1 week ago
On 3/3/26 5:07 PM, Farhan Ali wrote:
> 
> On 2/12/2026 12:43 PM, Zhuoying Cai wrote:
>> +static bool is_comp_overlap(SecureIplCompAddrRange *comp_addr_range,
>> +                            int addr_range_index,
>> +                            uint64_t start_addr, uint64_t end_addr)
>> +{
>> +    /* neither a signed nor an unsigned component can overlap with a signed component */
>> +    for (int i = 0; i < addr_range_index; i++) {
>> +        if ((comp_addr_range[i].start_addr < end_addr &&
>> +            start_addr < comp_addr_range[i].end_addr) &&
>> +            comp_addr_range[i].is_signed) {
>> +            return true;
>> +       }
>> +    }
>> +
>> +    return false;
>> +}
>> +
> 
> Shouldn't we use <= and >= checks? For example if 
> comp_addr_range[i].start_addr == end_addr, wouldn't that be an overlap? 
> and similar for start_addr?
>
> Thanks
> 
> Farhan
> 
>

Please correct me if I’m wrong. My understanding is that the component
address ranges are defined as half-open intervals [start_addr,
end_addr), where end_addr is exclusive. If that is the case, then the <
comparisons are correct — for example, [1, 3) and [3, 5) are adjacent
but not overlapping.






Re: [PATCH v8 20/30] pc-bios/s390-ccw: Add signed component address overlap checks
Posted by Thomas Huth 1 month, 1 week ago
On 12/02/2026 21.43, Zhuoying Cai wrote:
> Add address range tracking and overlap checks to ensure that no
> component overlaps with a signed component during secure IPL.
> 
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>   pc-bios/s390-ccw/secure-ipl.c | 54 +++++++++++++++++++++++++++++++++++
>   pc-bios/s390-ccw/secure-ipl.h |  6 ++++
>   2 files changed, 60 insertions(+)
> 
> diff --git a/pc-bios/s390-ccw/secure-ipl.c b/pc-bios/s390-ccw/secure-ipl.c
> index 1d5c2d40ce..27d2833642 100644
> --- a/pc-bios/s390-ccw/secure-ipl.c
> +++ b/pc-bios/s390-ccw/secure-ipl.c
> @@ -210,6 +210,55 @@ static void init_lists(IplDeviceComponentList *comp_list,
>       cert_list->ipl_info_header.len = sizeof(cert_list->ipl_info_header);
>   }
>   
> +static bool is_comp_overlap(SecureIplCompAddrRange *comp_addr_range,
> +                            int addr_range_index,
> +                            uint64_t start_addr, uint64_t end_addr)
> +{
> +    /* neither a signed nor an unsigned component can overlap with a signed component */
> +    for (int i = 0; i < addr_range_index; i++) {
> +        if ((comp_addr_range[i].start_addr < end_addr &&
> +            start_addr < comp_addr_range[i].end_addr) &&
> +            comp_addr_range[i].is_signed) {
> +            return true;
> +       }
> +    }
> +
> +    return false;
> +}
> +
> +static void comp_addr_range_add(SecureIplCompAddrRange *comp_addr_range,
> +                                int addr_range_index, bool is_signed,
> +                                uint64_t start_addr, uint64_t end_addr)
> +{
> +    if (addr_range_index >= MAX_CERTIFICATES) {
> +        printf("Warning: Ignoring component address range index [%d]"
> +               " because the maximum supported index is %d\n",
> +               addr_range_index, MAX_CERTIFICATES - 1);
> +        return;

Shouldn't this rather panic() instead - otherwise we might continue boot in 
secure mode though there is a problem with overlapping components?

> +    }
> +
> +    comp_addr_range[addr_range_index].is_signed = is_signed;
> +    comp_addr_range[addr_range_index].start_addr = start_addr;
> +    comp_addr_range[addr_range_index].end_addr = end_addr;
> +}

  Thomas