[PATCH 1/4] xen/arm: dm: Bail out if padding != 0 for XEN_DMOP_set_irq_level

Michal Orzel posted 4 patches 8 months ago
[PATCH 1/4] xen/arm: dm: Bail out if padding != 0 for XEN_DMOP_set_irq_level
Posted by Michal Orzel 8 months ago
XEN_DMOP_set_irq_level operation requires elements of pad array (being
member of xen_dm_op_set_irq_level structure) to be 0. While handling the
hypercall we validate this. If one of the elements is not zero, we set
rc to -EINVAL. At this point we should stop further DM handling and bail
out propagating the error to the caller. However, instead of goto the
code uses break which has basically no meaningful effect. The rc value
is never read and the code continues with the hypercall processing ending
up (possibly) with the interrupt injection. Fix it.

Fixes: 5d752df85f2c ("xen/dm: Introduce xendevicemodel_set_irq_level DM op")
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
 xen/arch/arm/dm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/dm.c b/xen/arch/arm/dm.c
index 773a0a259272..fdb3d967ec98 100644
--- a/xen/arch/arm/dm.c
+++ b/xen/arch/arm/dm.c
@@ -93,7 +93,7 @@ int dm_op(const struct dmop_args *op_args)
             if ( data->pad[i] )
             {
                 rc = -EINVAL;
-                break;
+                goto out;
             }
         }
 
-- 
2.25.1
Re: [PATCH 1/4] xen/arm: dm: Bail out if padding != 0 for XEN_DMOP_set_irq_level
Posted by Bertrand Marquis 8 months ago
Hi Michal,

> On 3 Mar 2025, at 09:56, Michal Orzel <michal.orzel@amd.com> wrote:
> 
> XEN_DMOP_set_irq_level operation requires elements of pad array (being
> member of xen_dm_op_set_irq_level structure) to be 0. While handling the
> hypercall we validate this. If one of the elements is not zero, we set
> rc to -EINVAL. At this point we should stop further DM handling and bail
> out propagating the error to the caller. However, instead of goto the
> code uses break which has basically no meaningful effect. The rc value
> is never read and the code continues with the hypercall processing ending
> up (possibly) with the interrupt injection. Fix it.
> 
> Fixes: 5d752df85f2c ("xen/dm: Introduce xendevicemodel_set_irq_level DM op")
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand


> ---
> xen/arch/arm/dm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/dm.c b/xen/arch/arm/dm.c
> index 773a0a259272..fdb3d967ec98 100644
> --- a/xen/arch/arm/dm.c
> +++ b/xen/arch/arm/dm.c
> @@ -93,7 +93,7 @@ int dm_op(const struct dmop_args *op_args)
>             if ( data->pad[i] )
>             {
>                 rc = -EINVAL;
> -                break;
> +                goto out;
>             }
>         }
> 
> -- 
> 2.25.1
>