* [PATCH] iommu: intel: Apply the device scope length check to every device scope walk
@ 2026-10-12 8:41 Shirong Zhao
2026-10-16 0:45 ` Baolu Lu
0 siblings, 1 reply; 2+ messages in thread
From: Shirong Zhao @ 2026-10-12 8:41 UTC (permalink / raw)
To: dwmw2, baolu.lu, joro, will; +Cc: robin.murphy, iommu, linux-kernel
dmar_walk_remapping_entries() already refuses to step by a zero length:
next = (void *)iter + iter->length;
if (iter->length == 0) {
/* Avoid looping forever on bad ACPI tables */
pr_debug(FW_BUG "Invalid 0-length structure\n");
break;
}
Five walks over the same DMAR structures do not:
dmar_alloc_dev_scope() - counts namespace/endpoint/bridge scopes
dmar_insert_dev_scope() - attaches a scope to a PCI path
dmar_acpi_insert_dev_scope() - same, by ANDD enumeration id
dmar_acpi_dev_scope_init() - the ANDD entries at the end of the table
ir_parse_ioapic_hpet_scope() - IOAPIC and HPET scopes
Each advances its cursor by a length read out of the DMAR table, and
nothing constrains it. A zero length leaves the cursor where it is and
the walk never ends; a short one steps through table payload a byte at a
time, reinterpreting it as scopes. dmar_alloc_dev_scope() and
dmar_acpi_dev_scope_init() run on every boot with the IOMMU enabled, and a
short length in the latter hands andd->device_name to acpi_get_handle().
Use the check dmar_walk_remapping_entries() already uses.
Checked against torvalds/linux at HEAD; both files are unmodified there.
Found by EFSM.R17, which reports a loop whose cursor advances by a step
memory can supply as zero while the exit condition depends on that cursor.
Signed-off-by: Shirong Zhao <shxzhaosr@163.com>
---
drivers/iommu/intel/dmar.c | 16 ++++++++++++++++
drivers/iommu/intel/irq_remapping.c | 5 +++++
2 files changed, 21 insertions(+)
diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index ba675b0..1615ef6 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -94,6 +94,10 @@ void *dmar_alloc_dev_scope(void *start, void *end, int *cnt)
scope->entry_type != ACPI_DMAR_SCOPE_TYPE_HPET) {
pr_warn("Unsupported device scope\n");
}
+ if (!scope->length) {
+ pr_warn(FW_BUG "Invalid 0-length device scope\n");
+ break;
+ }
start += scope->length;
}
if (*cnt == 0)
@@ -227,6 +231,10 @@ int dmar_insert_dev_scope(struct dmar_pci_notify_info *info,
for (; start < end; start += scope->length) {
scope = start;
+ if (!scope->length) {
+ pr_warn(FW_BUG "Invalid 0-length device scope\n");
+ break;
+ }
if (scope->entry_type != ACPI_DMAR_SCOPE_TYPE_ENDPOINT &&
scope->entry_type != ACPI_DMAR_SCOPE_TYPE_BRIDGE)
continue;
@@ -743,6 +751,10 @@ static void __init dmar_acpi_insert_dev_scope(u8 device_number,
for (scope = (void *)(drhd + 1);
(unsigned long)scope < ((unsigned long)drhd) + drhd->header.length;
scope = ((void *)scope) + scope->length) {
+ if (!scope->length) {
+ pr_warn(FW_BUG "Invalid 0-length device scope\n");
+ break;
+ }
if (scope->entry_type != ACPI_DMAR_SCOPE_TYPE_NAMESPACE)
continue;
if (scope->enumeration_id != device_number)
@@ -778,6 +790,10 @@ static int __init dmar_acpi_dev_scope_init(void)
for (andd = (void *)dmar_tbl + sizeof(struct acpi_table_dmar);
((unsigned long)andd) < ((unsigned long)dmar_tbl) + dmar_tbl->length;
andd = ((void *)andd) + andd->header.length) {
+ if (!andd->header.length) {
+ pr_warn(FW_BUG "Invalid 0-length namespace entry\n");
+ break;
+ }
if (andd->header.type == ACPI_DMAR_TYPE_NAMESPACE) {
acpi_handle h;
struct acpi_device *adev;
diff --git a/drivers/iommu/intel/irq_remapping.c b/drivers/iommu/intel/irq_remapping.c
index 25c26f7..884347c 100644
--- a/drivers/iommu/intel/irq_remapping.c
+++ b/drivers/iommu/intel/irq_remapping.c
@@ -933,6 +933,11 @@ static int ir_parse_ioapic_hpet_scope(struct acpi_dmar_header *header,
ret = ir_parse_one_ioapic_scope(scope, iommu, drhd);
else if (scope->entry_type == ACPI_DMAR_SCOPE_TYPE_HPET)
ret = ir_parse_one_hpet_scope(scope, iommu, drhd);
+ if (!scope->length) {
+ pr_warn(FW_BUG "Invalid 0-length device scope\n");
+ ret = -EINVAL;
+ break;
+ }
start += scope->length;
}
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH] iommu: intel: Apply the device scope length check to every device scope walk
2026-10-12 8:41 [PATCH] iommu: intel: Apply the device scope length check to every device scope walk Shirong Zhao
@ 2026-10-16 0:45 ` Baolu Lu
0 siblings, 0 replies; 2+ messages in thread
From: Baolu Lu @ 2026-10-16 0:45 UTC (permalink / raw)
To: Shirong Zhao, dwmw2, joro, will
Cc: baolu.lu, robin.murphy, iommu, linux-kernel
On 10/12/2026 4:41 PM, Shirong Zhao wrote:
> dmar_walk_remapping_entries() already refuses to step by a zero length:
>
> next = (void *)iter + iter->length;
> if (iter->length == 0) {
> /* Avoid looping forever on bad ACPI tables */
> pr_debug(FW_BUG "Invalid 0-length structure\n");
> break;
> }
>
> Five walks over the same DMAR structures do not:
>
> dmar_alloc_dev_scope() - counts namespace/endpoint/bridge scopes
> dmar_insert_dev_scope() - attaches a scope to a PCI path
> dmar_acpi_insert_dev_scope() - same, by ANDD enumeration id
> dmar_acpi_dev_scope_init() - the ANDD entries at the end of the table
> ir_parse_ioapic_hpet_scope() - IOAPIC and HPET scopes
>
> Each advances its cursor by a length read out of the DMAR table, and
> nothing constrains it. A zero length leaves the cursor where it is and
> the walk never ends; a short one steps through table payload a byte at a
> time, reinterpreting it as scopes. dmar_alloc_dev_scope() and
> dmar_acpi_dev_scope_init() run on every boot with the IOMMU enabled, and a
> short length in the latter hands andd->device_name to acpi_get_handle().
>
> Use the check dmar_walk_remapping_entries() already uses.
>
> Checked against torvalds/linux at HEAD; both files are unmodified there.
>
> Found by EFSM.R17, which reports a loop whose cursor advances by a step
> memory can supply as zero while the exit condition depends on that cursor.
> Signed-off-by: Shirong Zhao<shxzhaosr@163.com>
>
> ---
> drivers/iommu/intel/dmar.c | 16 ++++++++++++++++
> drivers/iommu/intel/irq_remapping.c | 5 +++++
> 2 files changed, 21 insertions(+)
A zero length is never valid under the VT-d spec, so such a DMAR table
is a firmware bug.
The existing check in dmar_walk_remapping_entries() already rejects a
zero-length top-level structure before any of these walks run, and I'm
not aware of any shipping platform with a malformed device scope.
Without a real-world report, I'd rather not add more checks for this.
Thanks for looking at this, though.
Thanks,
baolu
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-16 0:45 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-12 8:41 [PATCH] iommu: intel: Apply the device scope length check to every device scope walk Shirong Zhao
2026-10-16 0:45 ` Baolu Lu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.