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

Aneesh Kumar K.V <[email protected]>
Newsgroups dev.linux.lists.linux-coco,dev.linux.lists.kvmarm,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
Kameron Carr <[email protected]> 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.