| Message ID | 20220522165900.83104-1-julien@xen.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/iommu: dt: Check the return value of xsm_deassign_dtdevice() | expand |
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>
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,
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 > >
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,
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;