On 5/5/26 16:18, 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 <[email protected]> > --- > pc-bios/s390-ccw/secure-ipl.c | 49 +++++++++++++++++++++++++++++++++++ > pc-bios/s390-ccw/secure-ipl.h | 15 +++++++++++ > 2 files changed, 64 insertions(+) > > diff --git a/pc-bios/s390-ccw/secure-ipl.c b/pc-bios/s390-ccw/secure-ipl.c > index 6e943446a7..fdac74aa97 100644 > --- a/pc-bios/s390-ccw/secure-ipl.c > +++ b/pc-bios/s390-ccw/secure-ipl.c > @@ -220,6 +220,51 @@ static void init_lists(IplDeviceComponentList *comp_list, > cert_list->ipl_info_header.len = sizeof(IplInfoBlockHeader); > } > > +static void check_comp_overlap(SecureIplCompAddrRangeList *range_list, > + SecureIplCompEntryInfo comp_entry_info) > +{ > + uint64_t start_addr; > + uint64_t end_addr; > + > + start_addr = comp_entry_info.addr; > + end_addr = comp_entry_info.addr + comp_entry_info.len; > + > + /* > + * Check component's address range does not overlap with any > + * signed component's address range. > + */ > + for (int i = 0; i < range_list->num; i++) { > + if (range_list->comp_addr_range[i].is_signed && > + (range_list->comp_addr_range[i].start_addr < end_addr && > + start_addr < range_list->comp_addr_range[i].end_addr)) { > + zipl_secure_error("Component addresses overlap"); > + } > + } > +} > + > +static void comp_addr_range_add(SecureIplCompAddrRangeList *range_list, > + SecureIplCompEntryInfo comp_entry_info, > + bool is_signed) > +{ > + uint64_t start_addr; > + uint64_t end_addr; > + > + start_addr = comp_entry_info.addr; > + end_addr = comp_entry_info.addr + comp_entry_info.len; > + > + if (range_list->num >= MAX_COMP_ENTRIES) { > + zipl_secure_error("Component address range update failed due to > out-of-range" > + " index; Overlapping validation cannot be > guaranteed"); > + return; > + } > + > + range_list->comp_addr_range[range_list->num].is_signed = is_signed; > + range_list->comp_addr_range[range_list->num].start_addr = start_addr; > + range_list->comp_addr_range[range_list->num].end_addr = end_addr; > + > + range_list->num += 1; > +} > + > static int zipl_load_signature(ComponentEntry *entry, uint64_t sig) > { > if (entry->compdat.sig_info.format != DER_SIGNATURE_FORMAT) { > @@ -261,6 +306,7 @@ 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 }; > + SecureIplCompAddrRangeList range_list = { 0 }; > int signed_count = 0; > > init_lists(&comp_list, &cert_list); > @@ -292,6 +338,9 @@ int zipl_run_secure(ComponentEntry **entry_ptr, uint8_t > *tmp_sec) > comp_entry_info.addr = comp_addr; > comp_entry_info.len = (uint64_t)comp_len; > > + check_comp_overlap(&range_list, comp_entry_info); > + comp_addr_range_add(&range_list, comp_entry_info, !!sig_len); > + > /* no signature present (unsigned component) */ > if (!sig_entry_info.len) { > break; > diff --git a/pc-bios/s390-ccw/secure-ipl.h b/pc-bios/s390-ccw/secure-ipl.h > index cc0302f56b..29bbf65c6c 100644 > --- a/pc-bios/s390-ccw/secure-ipl.h > +++ b/pc-bios/s390-ccw/secure-ipl.h > @@ -24,6 +24,21 @@ typedef struct SecureIplCompEntryInfo { > uint8_t flags; > } SecureIplCompEntryInfo; > > +typedef struct SecureIplCompAddrRange { > + bool is_signed; > + uint64_t start_addr; > + uint64_t end_addr; > +} SecureIplCompAddrRange; > + > +/* > + * Custom struct for managing a list of secure IPL component address ranges. > + * Tracks up to MAX_COMP_ENTRIES address ranges with an num counter. > + */ > +typedef struct SecureIplCompAddrRangeList { > + SecureIplCompAddrRange comp_addr_range[MAX_COMP_ENTRIES]; > + int num; > +} SecureIplCompAddrRangeList; > + > static inline void zipl_secure_error(const char *message) > { > switch (boot_mode) {
I wonder if these checks are necessary in the BIOS, since it looks like the kernel will handle it in arch/s390/boot/ipl_report.c and physmem_info.c? I guess confirmation is needed to check if component address are checked by the kernel. If it's still needed, perhaps model the code after how the `intersect` function works to simplify things a bit. Otherwise, I think it's safe to drop this patch and let the kernel manage checking the memory overlaps. -- Regards, Collin
