[PATCH] xen/iommu: dt: Check the return value of xsm_deassign_dtdevice()

Julien Grall posted 1 patch 1 year, 11 months ago
Test gitlab-ci passed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20220522165900.83104-1-julien@xen.org
xen/drivers/passthrough/device_tree.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH] xen/iommu: dt: Check the return value of xsm_deassign_dtdevice()
Posted by Julien Grall 1 year, 11 months ago
From: Julien Grall <jgrall@amazon.com>

xsm_deasign_dtdevice() will indicate whether the caller is allowed
to issue the operation. So the return value has to be checked.

Spotted by clang static analyzer.

Fixes: fe36cccc483c ("xen/passthrough: Extend XEN_DOMCTL_*assign_device to support DT device")
Signed-off-by: Julien Grall <jgrall@amazon.com>

---

Platform device-passthrough is not security supported. Hence why this is
sent directly to xen-devel.
---
 xen/drivers/passthrough/device_tree.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/drivers/passthrough/device_tree.c b/xen/drivers/passthrough/device_tree.c
index 98f2aa0dad68..1c32d7b50cce 100644
--- a/xen/drivers/passthrough/device_tree.c
+++ b/xen/drivers/passthrough/device_tree.c
@@ -269,6 +269,8 @@ int iommu_do_dt_domctl(struct xen_domctl *domctl, struct domain *d,
             break;
 
         ret = xsm_deassign_dtdevice(XSM_HOOK, d, dt_node_full_name(dev));
+        if ( ret )
+            break;
 
         if ( d == dom_io )
             return -EINVAL;
-- 
2.32.0
Re: [PATCH] xen/iommu: dt: Check the return value of xsm_deassign_dtdevice()
Posted by Bertrand Marquis 1 year, 11 months ago
Hi Julien,

> On 22 May 2022, at 17:59, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> xsm_deasign_dtdevice() will indicate whether the caller is allowed
> to issue the operation. So the return value has to be checked.
> 
> Spotted by clang static analyzer.
> 
> Fixes: fe36cccc483c ("xen/passthrough: Extend XEN_DOMCTL_*assign_device to support DT device")
> Signed-off-by: Julien Grall <jgrall@amazon.com>

With the typo spotted by Michal solved (can be done on commit):
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> 
> ---
> 
> Platform device-passthrough is not security supported. Hence why this is
> sent directly to xen-devel.
> ---
> xen/drivers/passthrough/device_tree.c | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/xen/drivers/passthrough/device_tree.c b/xen/drivers/passthrough/device_tree.c
> index 98f2aa0dad68..1c32d7b50cce 100644
> --- a/xen/drivers/passthrough/device_tree.c
> +++ b/xen/drivers/passthrough/device_tree.c
> @@ -269,6 +269,8 @@ int iommu_do_dt_domctl(struct xen_domctl *domctl, struct domain *d,
>             break;
> 
>         ret = xsm_deassign_dtdevice(XSM_HOOK, d, dt_node_full_name(dev));
> +        if ( ret )
> +            break;
> 
>         if ( d == dom_io )
>             return -EINVAL;
> -- 
> 2.32.0
> 
> 
Re: [PATCH] xen/iommu: dt: Check the return value of xsm_deassign_dtdevice()
Posted by Julien Grall 1 year, 11 months ago

On 24/05/2022 08:54, Bertrand Marquis wrote:
> Hi Julien,

Hi Bertrand,

>> On 22 May 2022, at 17:59, Julien Grall <julien@xen.org> wrote:
>>
>> From: Julien Grall <jgrall@amazon.com>
>>
>> xsm_deasign_dtdevice() will indicate whether the caller is allowed
>> to issue the operation. So the return value has to be checked.
>>
>> Spotted by clang static analyzer.
>>
>> Fixes: fe36cccc483c ("xen/passthrough: Extend XEN_DOMCTL_*assign_device to support DT device")
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
> With the typo spotted by Michal solved (can be done on commit):
> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

I fixed the typo and committed.

Cheers,

-- 
Julien Grall
Re: [PATCH] xen/iommu: dt: Check the return value of xsm_deassign_dtdevice()
Posted by Michal Orzel 1 year, 11 months ago
Hi Julien,

On 22.05.2022 18:59, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> xsm_deasign_dtdevice() will indicate whether the caller is allowed
s/deasign/deassign/

> to issue the operation. So the return value has to be checked.
> 
> Spotted by clang static analyzer.
> 
> Fixes: fe36cccc483c ("xen/passthrough: Extend XEN_DOMCTL_*assign_device to support DT device")
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Apart from that:
Reviewed-by: Michal Orzel <michal.orzel@arm.com>
Re: [PATCH] xen/iommu: dt: Check the return value of xsm_deassign_dtdevice()
Posted by Julien Grall 1 year, 11 months ago
On 23/05/2022 08:00, Michal Orzel wrote:
> Hi Julien,
Hi Michal,


> On 22.05.2022 18:59, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> xsm_deasign_dtdevice() will indicate whether the caller is allowed
> s/deasign/deassign/

Good spot! I will fix it on commit unless there are any objections.

> 
>> to issue the operation. So the return value has to be checked.
>>
>> Spotted by clang static analyzer.
>>
>> Fixes: fe36cccc483c ("xen/passthrough: Extend XEN_DOMCTL_*assign_device to support DT device")
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
> Apart from that:
> Reviewed-by: Michal Orzel <michal.orzel@arm.com>

Thanks!

Cheers,

-- 
Julien Grall