On 7/29/25 14:18, Zhuoying Cai wrote:
> Thank you for the feedback!
>
> On 7/28/25 4:59 PM, Collin Walling wrote:
>> On 7/11/25 17:10, Zhuoying Cai wrote:
>
> ...
>
[...]
>>
>> You need a check somewhere for no certs found in either the specified
>> range or no certs exist in the store at all:
>> - VCB output len = 64
>> - stored and remaining count = 0
>> - response code 0x0001
>>
>>> + if (in_len % TARGET_PAGE_SIZE != 0) {
>>> + return DIAG_320_RC_INVAL_VCB_LEN;
>>> + }
>>> +
>>> + if (first_vc_index > last_vc_index) {
>>> + return DIAG_320_RC_BAD_RANGE;
>>> + }
>>> +
>>> + if (first_vc_index == 0) {
>>> + /*
>>> + * Zero is a valid index for the first and last VC index.
>>> + * Zero index results in the VCB header and zero certificates
>>> returned.
>>> + */
>>> + if (last_vc_index == 0) {
>>> + goto out;
>>> + }
>>> +
>>> + /* DIAG320 certificate store remains a one origin for cert entries
>>> */
>>> + vcb->first_vc_index = 1;
>>> + first_vc_index = 1;
>>> + }
>>> +
>>> + vce_offset = VCB_HEADER_LEN;
>>> + vcb->out_len = VCB_HEADER_LEN;
>>> + remaining_space = in_len - VCB_HEADER_LEN;
>>> +
>
> Re: check for no certs found in either the specified range or no certs
> exist.
>
> This case is already handled.
>
> vcb->out_len = VCB_HEADER_LEN is set outside the for loop. If the index
> is invalid, the loop won’t execute, and both the stored and remaining VC
> counts remain unchanged at 0.
>
>>> + for (int i = first_vc_index - 1; i < last_vc_index && i < qcs->count;
>>> i++) {
>>> + VCEntry *vce;
>>> + S390IPLCertificate qcert = qcs->certs[i];
>>> + /*
>>> + * Each VCE is word aligned.
>>> + * Each variable length field within the VCE is also word aligned.
>>> + */
>>> + keyid_buf_size = ROUND_UP(qcert.key_id_size, 4);
>>> + hash_buf_size = ROUND_UP(qcert.hash_size, 4);
>>> + cert_buf_size = ROUND_UP(qcert.der_size, 4);
>>> + vce_len = VCE_HEADER_LEN + cert_buf_size + keyid_buf_size +
>>> hash_buf_size;
>>
>> You could define & set the above four lines inside build_vce (or as
>> respective fields in the helper functions mentioned above).
>>
>> The remaining space check could be done after the vce has been built.
>>
>>> +
>>> + /*
>>> + * If there is no more space to store the cert,
>>> + * set the remaining verification cert count and
>>> + * break early.
>>> + */
>>> + if (remaining_space < vce_len) {
>>> + vcb->remain_ct = cpu_to_be16(last_vc_index - i);
>>> + break;
>>> + }
>>
>> You also need to check somewhere if there is enough space to store *at
>> least* the first cert in the range:
>> - VCB output len = 64
>> - stored count = 0
>> - remaining count = // however are remaining
>> - response code 0x0001
>>
>
> This case is also covered by the if statement above:
>
> if (remaining_space < vce_len) {
> vcb->remain_ct = cpu_to_be16(last_vc_index - i);
> break;
> }
>
> Response code 0x0001 is returned at the end of the function for both cases.
>
Sorry, I neglected to mentally trace some different loop variables to
see that these cases are indeed handled. Thanks for clarifying!
[...]
--
Regards,
Collin
