Re: [RFC PATCH v4 08/11] coco: guest: arm64: Verify DA evidence with RSI_VDEV_GET_INFO digests

[Aneesh Kumar K . V]

Kameron Carr <kameroncarr@xxxxxxxxxxxxxxxxxxx> writes:

> On 4/27/2026 1:28 AM, Aneesh Kumar K.V (Arm) wrote:
>
>> +struct rsi_vdevice_info {
>> +	union {
>> +		struct {
>> +			u64 flags;
>> +			u64 id_index;
>> +			union {
>> +				u8 hash_algo;
>> +				u64 padding0;
>> +			};
>> +			u64 lock_nonce;
>> +			u64 meas_nonce;
>> +			u64 report_nonce;
>
> Nit:
> I found the name `lock_nonce` confusing. I originally assumed this was the
> nonce passed in when calling RHI_DA_VDEV_GET_MEASUREMENTS. The new RSI spec
> (2.0 beta 3) names these lock_seq, meas_seq, report_seq. Consider matching
> these names since the values represent sequence counters and to
> differentiate them from the nonce used in RHI_DA_VDEV_GET_MEASUREMENTS.
>

Will switch to that name.

>
>> +			union {
>> +				u8 format_type;
>> +				u64 padding1;
>> +			};
>> +			u64 format_version;
>> +			union {
>> +				u8 state;
>> +				u64 padding2;
>> +			};
>> +
>> +		};
>> +		u8 padding3[0x80];
>> +	};

...

>> +static int cca_apply_evidence_report_range(struct pci_dev *pdev,
>> +		struct pci_tsm_mmio *mmio, bool map)
>> +{
>> +	int i, ret;
>> +	struct resource *res;
>> +	unsigned long mmio_flags = 0; /* non coherent, not limited order */
>> +	int vdev_id = rsi_vdev_id(pdev);
>> +	struct pci_tsm_mmio_entry *entry;
>> +	struct cca_guest_dsc *dsc = to_cca_guest_dsc(pdev);
>> +
>> +	for (i = 0; i < mmio->nr; i++) {
>
> In the tsm unlock code path cca_tsm_unlock() ->
> cca_unmap_evidence_report_range() -> cca_apply_evidence_report_range()
> there is no null pointer check on mmio / dsc->pci.mmio.
>
> dsc->pci.mmio is only initialized in the tsm accept path, so if the device
> is locked then unlocked without an accept, this will lead to a null pointer
> dereference.
>

I already have a fix for it in my development branch.


modified   drivers/virt/coco/arm-cca-guest/arm-cca.c
@@ -489,13 +489,15 @@ static void cca_tsm_unlock(struct pci_tsm *tsm)
 	}
 
 	cca_device_unlock(tsm->pdev);
-	pci_tsm_mmio_teardown(cca_dsc->pci.mmio);
+	if (cca_dsc->pci.mmio)
+		pci_tsm_mmio_teardown(cca_dsc->pci.mmio);
 
 err_out:
 	/*
 	 * No error handling from this function. Leave the device locked
 	 */
-	pci_tsm_mmio_free(tsm->pdev, cca_dsc->pci.mmio);
+	if (cca_dsc->pci.mmio)
+		pci_tsm_mmio_free(tsm->pdev, cca_dsc->pci.mmio);
 	kfree(cca_dsc);
 }
 
modified   drivers/virt/coco/arm-cca-guest/rsi-da.c
@@ -155,7 +155,9 @@ int cca_unmap_evidence_report_range(struct pci_dev *pdev)
 	struct cca_guest_dsc *dsc = to_cca_guest_dsc(pdev);
 	struct pci_tsm_mmio *tsm_mmio = dsc->pci.mmio;
 
-	return cca_apply_evidence_report_range(pdev, tsm_mmio, false);
+	if (tsm_mmio)
+		return cca_apply_evidence_report_range(pdev, tsm_mmio, false);
+	return 0;
 }
 
 int cca_verify_digest(u64 hash_algo, uint8_t *report,


-aneesh