The response code from user space is only allowed to be SUCCESS or
INVALID. All other values are treated by the device as a response
code of Response Failure according to PCI spec, section 10.4.2.1.
This response disables the Page Request Interface for the Function.
Add a check in iommufd_fault_fops_write() to avoid invalid response
code.
Fixes: 07838f7fd529 ("iommufd: Add iommufd fault object")
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
drivers/iommu/iommufd/fault.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/iommu/iommufd/fault.c b/drivers/iommu/iommufd/fault.c
index 54d6cd20a673..044b9b97da31 100644
--- a/drivers/iommu/iommufd/fault.c
+++ b/drivers/iommu/iommufd/fault.c
@@ -305,6 +305,12 @@ static ssize_t iommufd_fault_fops_write(struct file *filep, const char __user *b
if (rc)
break;
+ if (response.code != IOMMUFD_PAGE_RESP_SUCCESS &&
+ response.code != IOMMUFD_PAGE_RESP_INVALID) {
+ rc = -EINVAL;
+ break;
+ }
+
group = xa_erase(&fault->response, response.cookie);
if (!group) {
rc = -EINVAL;
--
2.34.1On Wed, Jul 10, 2024 at 04:33:40PM +0800, Lu Baolu wrote:
> The response code from user space is only allowed to be SUCCESS or
> INVALID. All other values are treated by the device as a response
> code of Response Failure according to PCI spec, section 10.4.2.1.
> This response disables the Page Request Interface for the Function.
>
> Add a check in iommufd_fault_fops_write() to avoid invalid response
> code.
>
> Fixes: 07838f7fd529 ("iommufd: Add iommufd fault object")
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
> drivers/iommu/iommufd/fault.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/iommu/iommufd/fault.c b/drivers/iommu/iommufd/fault.c
> index 54d6cd20a673..044b9b97da31 100644
> --- a/drivers/iommu/iommufd/fault.c
> +++ b/drivers/iommu/iommufd/fault.c
> @@ -305,6 +305,12 @@ static ssize_t iommufd_fault_fops_write(struct file *filep, const char __user *b
> if (rc)
> break;
>
> + if (response.code != IOMMUFD_PAGE_RESP_SUCCESS &&
> + response.code != IOMMUFD_PAGE_RESP_INVALID) {
> + rc = -EINVAL;
> + break;
> + }
I added this:
static_assert(IOMMUFD_PAGE_RESP_SUCCESS ==
IOMMU_PAGE_RESP_SUCCESS);
static_assert(IOMMUFD_PAGE_RESP_INVALID ==
IOMMU_PAGE_RESP_INVALID);
As well
Jason
On 7/12/24 7:39 AM, Jason Gunthorpe wrote:
> On Wed, Jul 10, 2024 at 04:33:40PM +0800, Lu Baolu wrote:
>> The response code from user space is only allowed to be SUCCESS or
>> INVALID. All other values are treated by the device as a response
>> code of Response Failure according to PCI spec, section 10.4.2.1.
>> This response disables the Page Request Interface for the Function.
>>
>> Add a check in iommufd_fault_fops_write() to avoid invalid response
>> code.
>>
>> Fixes: 07838f7fd529 ("iommufd: Add iommufd fault object")
>> Signed-off-by: Lu Baolu<baolu.lu@linux.intel.com>
>> ---
>> drivers/iommu/iommufd/fault.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/iommu/iommufd/fault.c b/drivers/iommu/iommufd/fault.c
>> index 54d6cd20a673..044b9b97da31 100644
>> --- a/drivers/iommu/iommufd/fault.c
>> +++ b/drivers/iommu/iommufd/fault.c
>> @@ -305,6 +305,12 @@ static ssize_t iommufd_fault_fops_write(struct file *filep, const char __user *b
>> if (rc)
>> break;
>>
>> + if (response.code != IOMMUFD_PAGE_RESP_SUCCESS &&
>> + response.code != IOMMUFD_PAGE_RESP_INVALID) {
>> + rc = -EINVAL;
>> + break;
>> + }
>
> I added this:
>
> static_assert(IOMMUFD_PAGE_RESP_SUCCESS ==
> IOMMU_PAGE_RESP_SUCCESS);
> static_assert(IOMMUFD_PAGE_RESP_INVALID ==
> IOMMU_PAGE_RESP_INVALID);
Above change cause below build warning:
drivers/iommu/iommufd/fault.c: In function ‘iommufd_fault_fops_write’:
drivers/iommu/iommufd/fault.c:308:57: warning: comparison between ‘enum
iommufd_page_response_code’ and ‘enum iommu_page_response_code’
[-Wenum-compare]
308 | static_assert(IOMMUFD_PAGE_RESP_SUCCESS ==
| ^~
./include/linux/build_bug.h:78:56: note: in definition of macro
‘__static_assert’
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
drivers/iommu/iommufd/fault.c:308:17: note: in expansion of macro
‘static_assert’
308 | static_assert(IOMMUFD_PAGE_RESP_SUCCESS ==
| ^~~~~~~~~~~~~
drivers/iommu/iommufd/fault.c:310:57: warning: comparison between ‘enum
iommufd_page_response_code’ and ‘enum iommu_page_response_code’
[-Wenum-compare]
310 | static_assert(IOMMUFD_PAGE_RESP_INVALID ==
| ^~
./include/linux/build_bug.h:78:56: note: in definition of macro
‘__static_assert’
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
drivers/iommu/iommufd/fault.c:310:17: note: in expansion of macro
‘static_assert’
310 | static_assert(IOMMUFD_PAGE_RESP_INVALID ==
| ^~~~~~~~~~~~~
Perhaps convert them to 'int' before compare?
diff --git a/drivers/iommu/iommufd/fault.c b/drivers/iommu/iommufd/fault.c
index 29f522819759..a643d5c7c535 100644
--- a/drivers/iommu/iommufd/fault.c
+++ b/drivers/iommu/iommufd/fault.c
@@ -305,10 +305,10 @@ static ssize_t iommufd_fault_fops_write(struct
file *filep, const char __user *b
if (rc)
break;
- static_assert(IOMMUFD_PAGE_RESP_SUCCESS ==
- IOMMU_PAGE_RESP_SUCCESS);
- static_assert(IOMMUFD_PAGE_RESP_INVALID ==
- IOMMU_PAGE_RESP_INVALID);
+ static_assert((int)IOMMUFD_PAGE_RESP_SUCCESS ==
+ (int)IOMMU_PAGE_RESP_SUCCESS);
+ static_assert((int)IOMMUFD_PAGE_RESP_INVALID ==
+ (int)IOMMU_PAGE_RESP_INVALID);
if (response.code != IOMMUFD_PAGE_RESP_SUCCESS &&
response.code != IOMMUFD_PAGE_RESP_INVALID) {
rc = -EINVAL;
Thanks,
baolu
On Fri, Jul 12, 2024 at 10:54:11AM +0800, Baolu Lu wrote:
> diff --git a/drivers/iommu/iommufd/fault.c b/drivers/iommu/iommufd/fault.c
> index 29f522819759..a643d5c7c535 100644
> --- a/drivers/iommu/iommufd/fault.c
> +++ b/drivers/iommu/iommufd/fault.c
> @@ -305,10 +305,10 @@ static ssize_t iommufd_fault_fops_write(struct file
> *filep, const char __user *b
> if (rc)
> break;
>
> - static_assert(IOMMUFD_PAGE_RESP_SUCCESS ==
> - IOMMU_PAGE_RESP_SUCCESS);
> - static_assert(IOMMUFD_PAGE_RESP_INVALID ==
> - IOMMU_PAGE_RESP_INVALID);
> + static_assert((int)IOMMUFD_PAGE_RESP_SUCCESS ==
> + (int)IOMMU_PAGE_RESP_SUCCESS);
> + static_assert((int)IOMMUFD_PAGE_RESP_INVALID ==
> + (int)IOMMU_PAGE_RESP_INVALID);
> if (response.code != IOMMUFD_PAGE_RESP_SUCCESS &&
> response.code != IOMMUFD_PAGE_RESP_INVALID) {
> rc = -EINVAL;
Yep, my compiler doesn't warn on that apparently..
Jason
> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Wednesday, July 10, 2024 4:34 PM
>
> The response code from user space is only allowed to be SUCCESS or
> INVALID. All other values are treated by the device as a response
> code of Response Failure according to PCI spec, section 10.4.2.1.
> This response disables the Page Request Interface for the Function.
>
> Add a check in iommufd_fault_fops_write() to avoid invalid response
> code.
"avoid invalid response code" but "RESP_INVALID is allowed" 😊.
I know what it means though...
>
> Fixes: 07838f7fd529 ("iommufd: Add iommufd fault object")
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
© 2016 - 2025 Red Hat, Inc.