[PATCH rc v3] iommufd: Fix struct iommu_hwpt_pgfault init and padding

From: Nicolin Chen
Date: Mon Jan 20 2025 - 14:51:49 EST


The iommu_hwpt_pgfault is used to report IO page fault data to userspace,
but iommufd_fault_fops_read was never zeroing its padding. This leaks the
content of the kernel stack memory to userspace.

Meanwhile, although the padding could be done by the compiler, explicitly
add a 32-bit padding. Also, change the __u64 addr to __aligned_u64.

pahole result, before:
struct iommu_hwpt_pgfault {
__u32 flags; /* 0 4 */
__u32 dev_id; /* 4 4 */
__u32 pasid; /* 8 4 */
__u32 grpid; /* 12 4 */
__u32 perm; /* 16 4 */

/* XXX 4 bytes hole, try to pack */

__u64 addr; /* 24 8 */
__u32 length; /* 32 4 */
__u32 cookie; /* 36 4 */

/* size: 40, cachelines: 1, members: 8 */
/* sum members: 36, holes: 1, sum holes: 4 */
/* last cacheline: 40 bytes */
};

pahole result, after:
struct iommu_hwpt_pgfault {
__u32 flags; /* 0 4 */
__u32 dev_id; /* 4 4 */
__u32 pasid; /* 8 4 */
__u32 grpid; /* 12 4 */
__u32 perm; /* 16 4 */
__u32 __reserved; /* 20 4 */
__u64 addr __attribute__((__aligned__(8))); /* 24 8 */
__u32 length; /* 32 4 */
__u32 cookie; /* 36 4 */

/* size: 40, cachelines: 1, members: 9 */
/* forced alignments: 1 */
/* last cacheline: 40 bytes */
} __attribute__((__aligned__(8)));

Fixes: c714f15860fc ("iommufd: Add fault and response message definitions")
Cc: stable@xxxxxxxxxxxxxxx
Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
Signed-off-by: Nicolin Chen <nicolinc@xxxxxxxxxx>
---
Changelog
v3
* Cc stable tree
* Fix iommu_hwpt_pgfault init in iommufd_fault_fops_read
* Drop the "__reserved = 0" in v2 since it's redundant now
v2
* Add "__reserved = 0" in iommufd_compose_fault_message

drivers/iommu/iommufd/fault.c | 2 +-
include/uapi/linux/iommufd.h | 4 +++-
2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/iommufd/fault.c b/drivers/iommu/iommufd/fault.c
index a9160f4443d2..d9a937450e55 100644
--- a/drivers/iommu/iommufd/fault.c
+++ b/drivers/iommu/iommufd/fault.c
@@ -263,7 +263,7 @@ static ssize_t iommufd_fault_fops_read(struct file *filep, char __user *buf,
{
size_t fault_size = sizeof(struct iommu_hwpt_pgfault);
struct iommufd_fault *fault = filep->private_data;
- struct iommu_hwpt_pgfault data;
+ struct iommu_hwpt_pgfault data = {};
struct iommufd_device *idev;
struct iopf_group *group;
struct iopf_fault *iopf;
diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
index 34810f6ae2b5..78747b24bd0f 100644
--- a/include/uapi/linux/iommufd.h
+++ b/include/uapi/linux/iommufd.h
@@ -868,6 +868,7 @@ enum iommu_hwpt_pgfault_perm {
* @pasid: Process Address Space ID
* @grpid: Page Request Group Index
* @perm: Combination of enum iommu_hwpt_pgfault_perm
+ * @__reserved: Must be 0.
* @addr: Fault address
* @length: a hint of how much data the requestor is expecting to fetch. For
* example, if the PRI initiator knows it is going to do a 10MB
@@ -883,7 +884,8 @@ struct iommu_hwpt_pgfault {
__u32 pasid;
__u32 grpid;
__u32 perm;
- __u64 addr;
+ __u32 __reserved;
+ __aligned_u64 addr;
__u32 length;
__u32 cookie;
};
--
2.34.1