Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] iommu/virtio: unaligned 64-bit access due to unvalidated property length --
commit 1966676bc77a49f0f207bc2272bebf67d2bd8f06 Author: Yehyeong Lee <[email protected]> iommu/virtio: bound the probe-property walk to the probe buffer This commit prevents an out-of-bounds read in viommu_probe_endpoint() by checking if the property fits within the probe allocation before dereferencing it. It also moves the length check in viommu_add_resv_mem() ahead of the memory accesses to avoid reading past the end of the allocation. > diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c > index 587fc13197f12..dbc997a6e3568 100644 > --- a/drivers/iommu/virtio-iommu.c > +++ b/drivers/iommu/virtio-iommu.c > @@ -489,6 +489,9 @@ static int viommu_add_resv_mem(struct viommu_endpoint > *vdev, > struct iommu_resv_region *region = NULL, *next; > unsigned long prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO; > > + if (len < sizeof(*mem)) > + return -EINVAL; > + > start = start64 = le64_to_cpu(mem->start); > end = end64 = le64_to_cpu(mem->end); [Severity: High] If a buggy or malicious device supplies an unaligned property length earlier in the parsing loop, could the mem pointer passed to viommu_add_resv_mem() be unaligned? Because the virtio_iommu_probe_resv_mem structure is not packed, the compiler assumes natural alignment. On architectures with strict alignment requirements (such as ARM or SPARC), wouldn't these direct 64-bit dereferences via le64_to_cpu() trigger a hardware alignment fault and lead to a kernel panic? [ ... ] > @@ -563,8 +564,10 @@ static int viommu_probe_endpoint(struct viommu_dev > *viommu, struct device *dev) > type = le16_to_cpu(prop->type) & VIRTIO_IOMMU_PROBE_T_MASK; > > while (type != VIRTIO_IOMMU_PROBE_T_NONE && > - cur < viommu->probe_size) { > + cur + sizeof(*prop) <= viommu->probe_size) { > len = le16_to_cpu(prop->length) + sizeof(*prop); > + if (len > viommu->probe_size - cur) > + break; [Severity: High] The virtio-iommu specification requires property lengths to be 8-byte aligned, but this length validation does not enforce alignment. Can an unaligned property length from the untrusted device advance the cur offset incorrectly? viommu_probe_endpoint() ... cur += len; Wouldn't this cause subsequent property structures to be mapped to unaligned memory addresses? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
