Skip to content

Commit 0e59645

Browse files
Guanghui Fengjoergroedel
authored andcommitted
iommu/amd: Fix illegal cap/mmio access in IOMMU debugfs
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> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
1 parent e4172c5 commit 0e59645

1 file changed

Lines changed: 19 additions & 23 deletions

File tree

drivers/iommu/amd/debugfs.c

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -26,37 +26,36 @@ static ssize_t iommu_mmio_write(struct file *filp, const char __user *ubuf,
2626
{
2727
struct seq_file *m = filp->private_data;
2828
struct amd_iommu *iommu = m->private;
29-
int ret;
30-
31-
iommu->dbg_mmio_offset = -1;
29+
int ret, dbg_mmio_offset = iommu->dbg_mmio_offset = -1;
3230

3331
if (cnt > OFS_IN_SZ)
3432
return -EINVAL;
3533

36-
ret = kstrtou32_from_user(ubuf, cnt, 0, &iommu->dbg_mmio_offset);
34+
ret = kstrtou32_from_user(ubuf, cnt, 0, &dbg_mmio_offset);
3735
if (ret)
3836
return ret;
3937

40-
if (iommu->dbg_mmio_offset > iommu->mmio_phys_end - sizeof(u64)) {
41-
iommu->dbg_mmio_offset = -1;
42-
return -EINVAL;
43-
}
38+
if (dbg_mmio_offset > iommu->mmio_phys_end - sizeof(u64))
39+
return -EINVAL;
4440

41+
iommu->dbg_mmio_offset = dbg_mmio_offset;
4542
return cnt;
4643
}
4744

4845
static int iommu_mmio_show(struct seq_file *m, void *unused)
4946
{
5047
struct amd_iommu *iommu = m->private;
5148
u64 value;
49+
int dbg_mmio_offset = iommu->dbg_mmio_offset;
5250

53-
if (iommu->dbg_mmio_offset < 0) {
51+
if (dbg_mmio_offset < 0 || dbg_mmio_offset >
52+
iommu->mmio_phys_end - sizeof(u64)) {
5453
seq_puts(m, "Please provide mmio register's offset\n");
5554
return 0;
5655
}
5756

58-
value = readq(iommu->mmio_base + iommu->dbg_mmio_offset);
59-
seq_printf(m, "Offset:0x%x Value:0x%016llx\n", iommu->dbg_mmio_offset, value);
57+
value = readq(iommu->mmio_base + dbg_mmio_offset);
58+
seq_printf(m, "Offset:0x%x Value:0x%016llx\n", dbg_mmio_offset, value);
6059

6160
return 0;
6261
}
@@ -67,45 +66,42 @@ static ssize_t iommu_capability_write(struct file *filp, const char __user *ubuf
6766
{
6867
struct seq_file *m = filp->private_data;
6968
struct amd_iommu *iommu = m->private;
70-
int ret;
71-
72-
iommu->dbg_cap_offset = -1;
69+
int ret, dbg_cap_offset = iommu->dbg_cap_offset = -1;
7370

7471
if (cnt > OFS_IN_SZ)
7572
return -EINVAL;
7673

77-
ret = kstrtou32_from_user(ubuf, cnt, 0, &iommu->dbg_cap_offset);
74+
ret = kstrtou32_from_user(ubuf, cnt, 0, &dbg_cap_offset);
7875
if (ret)
7976
return ret;
8077

8178
/* Capability register at offset 0x14 is the last IOMMU capability register. */
82-
if (iommu->dbg_cap_offset > 0x14) {
83-
iommu->dbg_cap_offset = -1;
79+
if (dbg_cap_offset > 0x14)
8480
return -EINVAL;
85-
}
8681

82+
iommu->dbg_cap_offset = dbg_cap_offset;
8783
return cnt;
8884
}
8985

9086
static int iommu_capability_show(struct seq_file *m, void *unused)
9187
{
9288
struct amd_iommu *iommu = m->private;
9389
u32 value;
94-
int err;
90+
int err, dbg_cap_offset = iommu->dbg_cap_offset;
9591

96-
if (iommu->dbg_cap_offset < 0) {
92+
if (dbg_cap_offset < 0 || dbg_cap_offset > 0x14) {
9793
seq_puts(m, "Please provide capability register's offset in the range [0x00 - 0x14]\n");
9894
return 0;
9995
}
10096

101-
err = pci_read_config_dword(iommu->dev, iommu->cap_ptr + iommu->dbg_cap_offset, &value);
97+
err = pci_read_config_dword(iommu->dev, iommu->cap_ptr + dbg_cap_offset, &value);
10298
if (err) {
10399
seq_printf(m, "Not able to read capability register at 0x%x\n",
104-
iommu->dbg_cap_offset);
100+
dbg_cap_offset);
105101
return 0;
106102
}
107103

108-
seq_printf(m, "Offset:0x%x Value:0x%08x\n", iommu->dbg_cap_offset, value);
104+
seq_printf(m, "Offset:0x%x Value:0x%08x\n", dbg_cap_offset, value);
109105

110106
return 0;
111107
}

0 commit comments

Comments
 (0)