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