On 7/1/26 4:48 PM, Zhuoying Cai wrote:
> Define MAX_VCENTRY_SIZE(8KB) and CERT_BUF_MAX_LEN to establish a finite
> size for a single entry VCEntry. Add validation in update_cert_store()
> to ensure certificate data does not exceed this limit.
>
> This finite size definition is needed for proper memory allocation and
> will be used in a later commit to handle VCEntry structures with known
> size constraints.
>
> Signed-off-by: Zhuoying Cai <[email protected]>
> ---
> hw/s390x/cert-store.c | 6 ++++++
> include/hw/s390x/ipl/diag320.h | 3 +++
> 2 files changed, 9 insertions(+)
>
> diff --git a/hw/s390x/cert-store.c b/hw/s390x/cert-store.c
> index ab3abf414b..08c38d18df 100644
> --- a/hw/s390x/cert-store.c
> +++ b/hw/s390x/cert-store.c
> @@ -86,6 +86,12 @@ static int update_cert_store(S390IPLCertificateStore
> *cert_store,
> cert_buf_size = ROUND_UP(cert->der_size, 4);
> data_buf_size = keyid_buf_size + hash_buf_size + cert_buf_size;
>
> + if (data_buf_size > CERT_BUF_MAX_LEN) {
> + error_report("Certificate data size %zu exceeds maximum buffer size
> %ld",
> + data_buf_size, CERT_BUF_MAX_LEN);
> + return -1;
> + }
> +
Took me a minute to realize why this needed to be its own patch vs
squashed in with hw/s390x/ipl: Create certificate store, because it
seems kind of like a fix? ...
> if (cert_store->largest_cert_size < data_buf_size) {
> cert_store->largest_cert_size = data_buf_size;
> }
> diff --git a/include/hw/s390x/ipl/diag320.h b/include/hw/s390x/ipl/diag320.h
> index 7fda2d44fd..f3c23a3176 100644
> --- a/include/hw/s390x/ipl/diag320.h
> +++ b/include/hw/s390x/ipl/diag320.h
> @@ -92,6 +92,9 @@ struct VCEntry {
> };
> typedef struct VCEntry VCEntry;
>
> +#define MAX_VCENTRY_SIZE (8 * 1024)
> +#define CERT_BUF_MAX_LEN (MAX_VCENTRY_SIZE - sizeof(VCEntryHeader))
> +
... it's because of VCEntryHeader. Would it be possible to instead pull
this code, the VCEntryHeader definition and CERT_NAME_MAX_LEN back
to 'hw/s390x/ipl: Create certificate store' so that we can detect
this condition right from the beginning?
Otherwise, the code itself seems fine so if it can't be squashed then:
Reviewed-by: Matthew Rosato <[email protected]>
> struct VCBlockHeader {
> uint32_t in_len;
> uint32_t reserved0;
