drivers/iommu/iommufd/fault.c | 1 + include/uapi/linux/iommufd.h | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-)
Though the padding could be done by the compiler, add a 32-bit padding
explicitly. Update iommufd_compose_fault_message to set it explicitly.
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")
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
drivers/iommu/iommufd/fault.c | 1 +
include/uapi/linux/iommufd.h | 4 +++-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/iommufd/fault.c b/drivers/iommu/iommufd/fault.c
index a9160f4443d2..52003bf1ba68 100644
--- a/drivers/iommu/iommufd/fault.c
+++ b/drivers/iommu/iommufd/fault.c
@@ -253,6 +253,7 @@ static void iommufd_compose_fault_message(struct iommu_fault *fault,
hwpt_fault->pasid = fault->prm.pasid;
hwpt_fault->grpid = fault->prm.grpid;
hwpt_fault->perm = fault->prm.perm;
+ hwpt_fault->__reserved = 0;
hwpt_fault->addr = fault->prm.addr;
hwpt_fault->length = 0;
hwpt_fault->cookie = cookie;
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.1On Sun, Jan 19, 2025 at 10:24:11PM -0800, Nicolin Chen wrote:
> Though the padding could be done by the compiler, add a 32-bit padding
> explicitly. Update iommufd_compose_fault_message to set it explicitly.
> Also, change the __u64 addr to __aligned_u64.
Mention that this also leaks the content of kernel stack memory to
userspace since it was never zeroing the padding.
> Fixes: c714f15860fc ("iommufd: Add fault and response message definitions")
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
> drivers/iommu/iommufd/fault.c | 1 +
> include/uapi/linux/iommufd.h | 4 +++-
> 2 files changed, 4 insertions(+), 1 deletion(-)
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Jason
On Sun, Jan 19, 2025 at 10:24:11PM -0800, Nicolin Chen wrote:
> --- a/drivers/iommu/iommufd/fault.c
> +++ b/drivers/iommu/iommufd/fault.c
> @@ -253,6 +253,7 @@ static void iommufd_compose_fault_message(struct iommu_fault *fault,
> hwpt_fault->pasid = fault->prm.pasid;
> hwpt_fault->grpid = fault->prm.grpid;
> hwpt_fault->perm = fault->prm.perm;
> + hwpt_fault->__reserved = 0;
> hwpt_fault->addr = fault->prm.addr;
> hwpt_fault->length = 0;
> hwpt_fault->cookie = cookie;
Yikes, so it was leaking kernel stack memory through the padding too.
We should zero init the stack struct to be safe:
@@ -247,7 +247,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;
I can fix it up if that is the only change
Jason
On Mon, Jan 20, 2025 at 10:37:19AM -0400, Jason Gunthorpe wrote:
> On Sun, Jan 19, 2025 at 10:24:11PM -0800, Nicolin Chen wrote:
> > --- a/drivers/iommu/iommufd/fault.c
> > +++ b/drivers/iommu/iommufd/fault.c
> > @@ -253,6 +253,7 @@ static void iommufd_compose_fault_message(struct iommu_fault *fault,
> > hwpt_fault->pasid = fault->prm.pasid;
> > hwpt_fault->grpid = fault->prm.grpid;
> > hwpt_fault->perm = fault->prm.perm;
> > + hwpt_fault->__reserved = 0;
> > hwpt_fault->addr = fault->prm.addr;
> > hwpt_fault->length = 0;
> > hwpt_fault->cookie = cookie;
>
> Yikes, so it was leaking kernel stack memory through the padding too.
>
> We should zero init the stack struct to be safe:
>
> @@ -247,7 +247,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;
>
> I can fix it up if that is the only change
Sending a v3.
I think having the zeroing in iommufd_fault_fops_read can drop
the "__reserved = 0" iommufd_compose_fault_message.
Thanks
Nicolin
© 2016 - 2026 Red Hat, Inc.