[PATCH 0/2] Fix illegal access in IOMMU debugfs

[PATCH 0/2] Fix illegal access in IOMMU debugfs

[Guanghui Feng]

In AMD IOMMU debugfs,
1. set the device, address, and other information to be accessed
2. After verifying the legitimacy of the device, address, and other
information in step 1, access and obtain the information

However, before actually accessing the device, invalid device or address
information might be set again after the legitimacy verification in step
2, thus triggering an unauthorized access issue.

Guanghui Feng (2):
  iommu/amd: Fix illegal device-id access in IOMMU debugfs
  iommu/amd: Fix illegal cap/mmio access in IOMMU debugfs

 drivers/iommu/amd/debugfs.c | 63 ++++++++++++++++++-------------------
 1 file changed, 31 insertions(+), 32 deletions(-)

-- 
2.43.7

[PATCH 1/2] iommu/amd: Fix illegal device-id access in IOMMU debugfs

[Guanghui Feng]

In the current AMD IOMMU debugFS, when multiple processes use the IOMMU
debugFS process simultaneously, illegal access issues can occur in the
following execution flow:

1. CPU1: Sets a valid sbdf via devid_write, then checks the sbdf's
validity in execution flows such as devid_show, iommu_devtbl_show,
and iommu_irqtbl_show.

2. CPU2: Sets an invalid sbdf via devid_write, at which point the sbdf
value is -1.

3. CPU1: accesses the IOMMU device table, IRQ table, based on the
invalid SBDF value of -1, resulting in illegal access.

This is especially problematic in monitoring scripts, where multiple
scripts may access debugFS simultaneously, and some scripts may
unexpectedly set invalid values, which triggers illegal access in
debugfs.

This patch modifies the execution flow of devid_show,
iommu_devtbl_show, and iommu_irqtbl_show to ensure that these
processes determine the validity and access based on the
same device-id, thus guaranteeing correctness and robustness.

Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
---
 drivers/iommu/amd/debugfs.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/amd/debugfs.c b/drivers/iommu/amd/debugfs.c
index 20b04996441d..0b03e0622f67 100644
--- a/drivers/iommu/amd/debugfs.c
+++ b/drivers/iommu/amd/debugfs.c
@@ -197,10 +197,11 @@ static ssize_t devid_write(struct file *filp, const char __user *ubuf,
 static int devid_show(struct seq_file *m, void *unused)
 {
 	u16 devid;
+	int sbdf_shadow = sbdf;
 
-	if (sbdf >= 0) {
-		devid = PCI_SBDF_TO_DEVID(sbdf);
-		seq_printf(m, "%04x:%02x:%02x.%x\n", PCI_SBDF_TO_SEGID(sbdf),
+	if (sbdf_shadow >= 0) {
+		devid = PCI_SBDF_TO_DEVID(sbdf_shadow);
+		seq_printf(m, "%04x:%02x:%02x.%x\n", PCI_SBDF_TO_SEGID(sbdf_shadow),
 			   PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid));
 	} else
 		seq_puts(m, "No or Invalid input provided\n");
@@ -237,13 +238,14 @@ static int iommu_devtbl_show(struct seq_file *m, void *unused)
 {
 	struct amd_iommu_pci_seg *pci_seg;
 	u16 seg, devid;
+	int sbdf_shadow = sbdf;
 
-	if (sbdf < 0) {
+	if (sbdf_shadow < 0) {
 		seq_puts(m, "Enter a valid device ID to 'devid' file\n");
 		return 0;
 	}
-	seg = PCI_SBDF_TO_SEGID(sbdf);
-	devid = PCI_SBDF_TO_DEVID(sbdf);
+	seg = PCI_SBDF_TO_SEGID(sbdf_shadow);
+	devid = PCI_SBDF_TO_DEVID(sbdf_shadow);
 
 	for_each_pci_segment(pci_seg) {
 		if (pci_seg->id != seg)
@@ -336,19 +338,20 @@ static int iommu_irqtbl_show(struct seq_file *m, void *unused)
 {
 	struct amd_iommu_pci_seg *pci_seg;
 	u16 devid, seg;
+	int sbdf_shadow = sbdf;
 
 	if (!irq_remapping_enabled) {
 		seq_puts(m, "Interrupt remapping is disabled\n");
 		return 0;
 	}
 
-	if (sbdf < 0) {
+	if (sbdf_shadow < 0) {
 		seq_puts(m, "Enter a valid device ID to 'devid' file\n");
 		return 0;
 	}
 
-	seg = PCI_SBDF_TO_SEGID(sbdf);
-	devid = PCI_SBDF_TO_DEVID(sbdf);
+	seg = PCI_SBDF_TO_SEGID(sbdf_shadow);
+	devid = PCI_SBDF_TO_DEVID(sbdf_shadow);
 
 	for_each_pci_segment(pci_seg) {
 		if (pci_seg->id != seg)
-- 
2.43.7

[PATCH 2/2] iommu/amd: Fix illegal cap/mmio access in IOMMU debugfs

[Guanghui Feng]

In the current AMD IOMMU debugfs, when multiple processes simultaneously
access the IOMMU mmio/cap registers using the IOMMU debugfs, illegal
access issues can occur in the following execution flow:

1. CPU1: Sets a valid access address using iommu_mmio/capability_write,
and verifies the access address's validity in iommu_mmio/capability_show

2. CPU2: Sets an invalid address using iommu_mmio/capability_write

3. CPU1: accesses the IOMMU mmio/cap registers based on the invalid
address, resulting in an illegal access.

This patch modifies the execution process to first verify the address's
validity and then access it based on the same address, ensuring
correctness and robustness.

Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
---
 drivers/iommu/amd/debugfs.c | 42 +++++++++++++++++--------------------
 1 file changed, 19 insertions(+), 23 deletions(-)

diff --git a/drivers/iommu/amd/debugfs.c b/drivers/iommu/amd/debugfs.c
index 0b03e0622f67..4e66473d7cea 100644
--- a/drivers/iommu/amd/debugfs.c
+++ b/drivers/iommu/amd/debugfs.c
@@ -26,22 +26,19 @@ static ssize_t iommu_mmio_write(struct file *filp, const char __user *ubuf,
 {
 	struct seq_file *m = filp->private_data;
 	struct amd_iommu *iommu = m->private;
-	int ret;
-
-	iommu->dbg_mmio_offset = -1;
+	int ret, dbg_mmio_offset = iommu->dbg_mmio_offset = -1;
 
 	if (cnt > OFS_IN_SZ)
 		return -EINVAL;
 
-	ret = kstrtou32_from_user(ubuf, cnt, 0, &iommu->dbg_mmio_offset);
+	ret = kstrtou32_from_user(ubuf, cnt, 0, &dbg_mmio_offset);
 	if (ret)
 		return ret;
 
-	if (iommu->dbg_mmio_offset > iommu->mmio_phys_end - sizeof(u64)) {
-		iommu->dbg_mmio_offset = -1;
-		return  -EINVAL;
-	}
+	if (dbg_mmio_offset > iommu->mmio_phys_end - sizeof(u64))
+		return -EINVAL;
 
+	iommu->dbg_mmio_offset = dbg_mmio_offset;
 	return cnt;
 }
 
@@ -49,14 +46,16 @@ static int iommu_mmio_show(struct seq_file *m, void *unused)
 {
 	struct amd_iommu *iommu = m->private;
 	u64 value;
+	int dbg_mmio_offset = iommu->dbg_mmio_offset;
 
-	if (iommu->dbg_mmio_offset < 0) {
+	if (dbg_mmio_offset < 0 || dbg_mmio_offset >
+			iommu->mmio_phys_end - sizeof(u64)) {
 		seq_puts(m, "Please provide mmio register's offset\n");
 		return 0;
 	}
 
-	value = readq(iommu->mmio_base + iommu->dbg_mmio_offset);
-	seq_printf(m, "Offset:0x%x Value:0x%016llx\n", iommu->dbg_mmio_offset, value);
+	value = readq(iommu->mmio_base + dbg_mmio_offset);
+	seq_printf(m, "Offset:0x%x Value:0x%016llx\n", dbg_mmio_offset, value);
 
 	return 0;
 }
@@ -67,23 +66,20 @@ static ssize_t iommu_capability_write(struct file *filp, const char __user *ubuf
 {
 	struct seq_file *m = filp->private_data;
 	struct amd_iommu *iommu = m->private;
-	int ret;
-
-	iommu->dbg_cap_offset = -1;
+	int ret, dbg_cap_offset = iommu->dbg_cap_offset = -1;
 
 	if (cnt > OFS_IN_SZ)
 		return -EINVAL;
 
-	ret = kstrtou32_from_user(ubuf, cnt, 0, &iommu->dbg_cap_offset);
+	ret = kstrtou32_from_user(ubuf, cnt, 0, &dbg_cap_offset);
 	if (ret)
 		return ret;
 
 	/* Capability register at offset 0x14 is the last IOMMU capability register. */
-	if (iommu->dbg_cap_offset > 0x14) {
-		iommu->dbg_cap_offset = -1;
+	if (dbg_cap_offset > 0x14)
 		return -EINVAL;
-	}
 
+	iommu->dbg_cap_offset = dbg_cap_offset;
 	return cnt;
 }
 
@@ -91,21 +87,21 @@ static int iommu_capability_show(struct seq_file *m, void *unused)
 {
 	struct amd_iommu *iommu = m->private;
 	u32 value;
-	int err;
+	int err, dbg_cap_offset = iommu->dbg_cap_offset;
 
-	if (iommu->dbg_cap_offset < 0) {
+	if (dbg_cap_offset < 0 || dbg_cap_offset > 0x14) {
 		seq_puts(m, "Please provide capability register's offset in the range [0x00 - 0x14]\n");
 		return 0;
 	}
 
-	err = pci_read_config_dword(iommu->dev, iommu->cap_ptr + iommu->dbg_cap_offset, &value);
+	err = pci_read_config_dword(iommu->dev, iommu->cap_ptr + dbg_cap_offset, &value);
 	if (err) {
 		seq_printf(m, "Not able to read capability register at 0x%x\n",
-			   iommu->dbg_cap_offset);
+			   dbg_cap_offset);
 		return 0;
 	}
 
-	seq_printf(m, "Offset:0x%x Value:0x%08x\n", iommu->dbg_cap_offset, value);
+	seq_printf(m, "Offset:0x%x Value:0x%08x\n", dbg_cap_offset, value);
 
 	return 0;
 }
-- 
2.43.7

Re: [PATCH 0/2] Fix illegal access in IOMMU debugfs

[Jörg Rödel]

On Thu, Mar 19, 2026 at 03:37:52PM +0800, Guanghui Feng wrote:
> In AMD IOMMU debugfs,
> 1. set the device, address, and other information to be accessed
> 2. After verifying the legitimacy of the device, address, and other
> information in step 1, access and obtain the information
> 
> However, before actually accessing the device, invalid device or address
> information might be set again after the legitimacy verification in step
> 2, thus triggering an unauthorized access issue.
> 
> Guanghui Feng (2):
>   iommu/amd: Fix illegal device-id access in IOMMU debugfs
>   iommu/amd: Fix illegal cap/mmio access in IOMMU debugfs
> 
>  drivers/iommu/amd/debugfs.c | 63 ++++++++++++++++++-------------------
>  1 file changed, 31 insertions(+), 32 deletions(-)

Applied, thanks.

Vasant, this patch-set fixes pretty serious issues. Can you please further
review the AMD IOMMU debugfs code to make it more robust and secure?

-Joerg

Re: [PATCH 1/2] iommu/amd: Fix illegal device-id access in IOMMU debugfs

[Vasant Hegde]

On 3/19/2026 1:07 PM, Guanghui Feng wrote:
> In the current AMD IOMMU debugFS, when multiple processes use the IOMMU
> debugFS process simultaneously, illegal access issues can occur in the
> following execution flow:
> 
> 1. CPU1: Sets a valid sbdf via devid_write, then checks the sbdf's
> validity in execution flows such as devid_show, iommu_devtbl_show,
> and iommu_irqtbl_show.
> 
> 2. CPU2: Sets an invalid sbdf via devid_write, at which point the sbdf
> value is -1.
> 
> 3. CPU1: accesses the IOMMU device table, IRQ table, based on the
> invalid SBDF value of -1, resulting in illegal access.
> 
> This is especially problematic in monitoring scripts, where multiple
> scripts may access debugFS simultaneously, and some scripts may
> unexpectedly set invalid values, which triggers illegal access in
> debugfs.
> 
> This patch modifies the execution flow of devid_show,
> iommu_devtbl_show, and iommu_irqtbl_show to ensure that these
> processes determine the validity and access based on the
> same device-id, thus guaranteeing correctness and robustness.
> 
 Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>

Looks good.

Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>

-Vasant

Re: [PATCH 2/2] iommu/amd: Fix illegal cap/mmio access in IOMMU debugfs

[Vasant Hegde]

On 3/19/2026 1:07 PM, Guanghui Feng wrote:
> In the current AMD IOMMU debugfs, when multiple processes simultaneously
> access the IOMMU mmio/cap registers using the IOMMU debugfs, illegal
> access issues can occur in the following execution flow:
> 
> 1. CPU1: Sets a valid access address using iommu_mmio/capability_write,
> and verifies the access address's validity in iommu_mmio/capability_show
> 
> 2. CPU2: Sets an invalid address using iommu_mmio/capability_write
> 
> 3. CPU1: accesses the IOMMU mmio/cap registers based on the invalid
> address, resulting in an illegal access.
> 
> This patch modifies the execution process to first verify the address's
> validity and then access it based on the same address, ensuring
> correctness and robustness.
> 
> Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>

Looks good.

Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>

-Vasant

Re: [PATCH 0/2] Fix illegal access in IOMMU debugfs

[guanghuifeng]

I am very willing to participate in the upstream community feature review.

Thanks.


在 2026/3/27 16:29, Jörg Rödel 写道:
> On Thu, Mar 19, 2026 at 03:37:52PM +0800, Guanghui Feng wrote:
>> In AMD IOMMU debugfs,
>> 1. set the device, address, and other information to be accessed
>> 2. After verifying the legitimacy of the device, address, and other
>> information in step 1, access and obtain the information
>>
>> However, before actually accessing the device, invalid device or address
>> information might be set again after the legitimacy verification in step
>> 2, thus triggering an unauthorized access issue.
>>
>> Guanghui Feng (2):
>>    iommu/amd: Fix illegal device-id access in IOMMU debugfs
>>    iommu/amd: Fix illegal cap/mmio access in IOMMU debugfs
>>
>>   drivers/iommu/amd/debugfs.c | 63 ++++++++++++++++++-------------------
>>   1 file changed, 31 insertions(+), 32 deletions(-)
> Applied, thanks.
>
> Vasant, this patch-set fixes pretty serious issues. Can you please further
> review the AMD IOMMU debugfs code to make it more robust and secure?
>
> -Joerg