1. Patchew
  2. Xen
  3. View series

[PATCH v2] arm/vgic-v2: Fix undefined behavior in vgic_fetch_itargetsr()

Jahan Murudi posted 1 patch 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20250601163212.2988162-1-jahan.murudi.zg@renesas.com
xen/arch/arm/vgic-v2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH v2] arm/vgic-v2: Fix undefined behavior in vgic_fetch_itargetsr()
Posted by Jahan Murudi 5 months ago 
The current implementation performs left shift operations that may trigger
undefined behavior when the target value is too large. This patch:

1. Changes the shift from signed (1) to unsigned (1U) to ensure well-defined
   behavior for all valid target values
2. Maintains identical functionality while fixing the UBSAN warning

The issue was detected by UBSAN:
(XEN) UBSAN: Undefined behaviour in arch/arm/vgic-v2.c:73:56
(XEN) left shift of 128 by 24 places cannot be represented in type 'int'
(XEN) Xen WARN at common/ubsan/ubsan.c:174

Signed-off-by: Jahan Murudi <jahan.murudi.zg@renesas.com>

---
Changed since v1:
 * Added space after subject line
---
 xen/arch/arm/vgic-v2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index a19d610178..642407fd5b 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -70,7 +70,7 @@ static uint32_t vgic_fetch_itargetsr(struct vgic_irq_rank *rank,
     offset &= ~(NR_TARGETS_PER_ITARGETSR - 1);
 
     for ( i = 0; i < NR_TARGETS_PER_ITARGETSR; i++, offset++ )
-        reg |= (1 << read_atomic(&rank->vcpu[offset])) << (i * NR_BITS_PER_TARGET);
+        reg |= (1U << read_atomic(&rank->vcpu[offset])) << (i * NR_BITS_PER_TARGET);
 
     return reg;
 }
-- 
2.34.1
Re: [PATCH v2] arm/vgic-v2: Fix undefined behavior in vgic_fetch_itargetsr()
Posted by Orzel, Michal 5 months ago 

On 01/06/2025 18:32, Jahan Murudi wrote:
> The current implementation performs left shift operations that may trigger
> undefined behavior when the target value is too large. This patch:
> 
> 1. Changes the shift from signed (1) to unsigned (1U) to ensure well-defined
NIT for the future: Use imperative mood

>    behavior for all valid target values
> 2. Maintains identical functionality while fixing the UBSAN warning
> 
> The issue was detected by UBSAN:
> (XEN) UBSAN: Undefined behaviour in arch/arm/vgic-v2.c:73:56
> (XEN) left shift of 128 by 24 places cannot be represented in type 'int'
> (XEN) Xen WARN at common/ubsan/ubsan.c:174
> 
> Signed-off-by: Jahan Murudi <jahan.murudi.zg@renesas.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal

> 
> ---
> Changed since v1:
>  * Added space after subject line
> ---
>  xen/arch/arm/vgic-v2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
> index a19d610178..642407fd5b 100644
> --- a/xen/arch/arm/vgic-v2.c
> +++ b/xen/arch/arm/vgic-v2.c
> @@ -70,7 +70,7 @@ static uint32_t vgic_fetch_itargetsr(struct vgic_irq_rank *rank,
>      offset &= ~(NR_TARGETS_PER_ITARGETSR - 1);
>  
>      for ( i = 0; i < NR_TARGETS_PER_ITARGETSR; i++, offset++ )
> -        reg |= (1 << read_atomic(&rank->vcpu[offset])) << (i * NR_BITS_PER_TARGET);
> +        reg |= (1U << read_atomic(&rank->vcpu[offset])) << (i * NR_BITS_PER_TARGET);
>  
>      return reg;
>  }
RE: [PATCH v2] arm/vgic-v2: Fix undefined behavior in vgic_fetch_itargetsr()
Posted by Jahan Murudi 4 months, 4 weeks ago 
 Hi Stefano/Julien,

This v2 patch was reviewed-by Michal Orzel on 2nd June. Could it be merged?

Link : https://patchwork.kernel.org/project/xen-devel/patch/20250601163212.2988162-1-jahan.murudi.zg@renesas.com/

Thanks, 
Jahan Murudi


-----Original Message-----
From: Orzel, Michal <michal.orzel@amd.com> 
Sent: 02 June 2025 12:24
To: Jahan Murudi <jahan.murudi.zg@renesas.com>; xen-devel@lists.xenproject.org
Cc: Stefano Stabellini <sstabellini@kernel.org>; Julien Grall <julien@xen.org>; Bertrand Marquis <bertrand.marquis@arm.com>; Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Subject: Re: [PATCH v2] arm/vgic-v2: Fix undefined behavior in vgic_fetch_itargetsr()



On 01/06/2025 18:32, Jahan Murudi wrote:
> The current implementation performs left shift operations that may 
> trigger undefined behavior when the target value is too large. This patch:
> 
> 1. Changes the shift from signed (1) to unsigned (1U) to ensure 
> well-defined
NIT for the future: Use imperative mood

>    behavior for all valid target values 2. Maintains identical 
> functionality while fixing the UBSAN warning
> 
> The issue was detected by UBSAN:
> (XEN) UBSAN: Undefined behaviour in arch/arm/vgic-v2.c:73:56
> (XEN) left shift of 128 by 24 places cannot be represented in type 'int'
> (XEN) Xen WARN at common/ubsan/ubsan.c:174
> 
> Signed-off-by: Jahan Murudi <jahan.murudi.zg@renesas.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal

> 
> ---
> Changed since v1:
>  * Added space after subject line
> ---
>  xen/arch/arm/vgic-v2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c index 
> a19d610178..642407fd5b 100644
> --- a/xen/arch/arm/vgic-v2.c
> +++ b/xen/arch/arm/vgic-v2.c
> @@ -70,7 +70,7 @@ static uint32_t vgic_fetch_itargetsr(struct vgic_irq_rank *rank,
>      offset &= ~(NR_TARGETS_PER_ITARGETSR - 1);
>  
>      for ( i = 0; i < NR_TARGETS_PER_ITARGETSR; i++, offset++ )
> -        reg |= (1 << read_atomic(&rank->vcpu[offset])) << (i * NR_BITS_PER_TARGET);
> +        reg |= (1U << read_atomic(&rank->vcpu[offset])) << (i * 
> + NR_BITS_PER_TARGET);
>  
>      return reg;
>  }
RE: [PATCH v2] arm/vgic-v2: Fix undefined behavior in vgic_fetch_itargetsr()
Posted by Stefano Stabellini 4 months, 4 weeks ago 
Yes, I committed it. Thank you for the ping.

Cheers,

Stefano

On Wed, 4 Jun 2025, Jahan Murudi wrote:
>  Hi Stefano/Julien,
> 
> This v2 patch was reviewed-by Michal Orzel on 2nd June. Could it be merged?
> 
> Link : https://patchwork.kernel.org/project/xen-devel/patch/20250601163212.2988162-1-jahan.murudi.zg@renesas.com/
> 
> Thanks, 
> Jahan Murudi
> 
> 
> -----Original Message-----
> From: Orzel, Michal <michal.orzel@amd.com> 
> Sent: 02 June 2025 12:24
> To: Jahan Murudi <jahan.murudi.zg@renesas.com>; xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini <sstabellini@kernel.org>; Julien Grall <julien@xen.org>; Bertrand Marquis <bertrand.marquis@arm.com>; Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> Subject: Re: [PATCH v2] arm/vgic-v2: Fix undefined behavior in vgic_fetch_itargetsr()
> 
> 
> 
> On 01/06/2025 18:32, Jahan Murudi wrote:
> > The current implementation performs left shift operations that may 
> > trigger undefined behavior when the target value is too large. This patch:
> > 
> > 1. Changes the shift from signed (1) to unsigned (1U) to ensure 
> > well-defined
> NIT for the future: Use imperative mood
> 
> >    behavior for all valid target values 2. Maintains identical 
> > functionality while fixing the UBSAN warning
> > 
> > The issue was detected by UBSAN:
> > (XEN) UBSAN: Undefined behaviour in arch/arm/vgic-v2.c:73:56
> > (XEN) left shift of 128 by 24 places cannot be represented in type 'int'
> > (XEN) Xen WARN at common/ubsan/ubsan.c:174
> > 
> > Signed-off-by: Jahan Murudi <jahan.murudi.zg@renesas.com>
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
> 
> ~Michal
> 
> > 
> > ---
> > Changed since v1:
> >  * Added space after subject line
> > ---
> >  xen/arch/arm/vgic-v2.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c index 
> > a19d610178..642407fd5b 100644
> > --- a/xen/arch/arm/vgic-v2.c
> > +++ b/xen/arch/arm/vgic-v2.c
> > @@ -70,7 +70,7 @@ static uint32_t vgic_fetch_itargetsr(struct vgic_irq_rank *rank,
> >      offset &= ~(NR_TARGETS_PER_ITARGETSR - 1);
> >  
> >      for ( i = 0; i < NR_TARGETS_PER_ITARGETSR; i++, offset++ )
> > -        reg |= (1 << read_atomic(&rank->vcpu[offset])) << (i * NR_BITS_PER_TARGET);
> > +        reg |= (1U << read_atomic(&rank->vcpu[offset])) << (i * 
> > + NR_BITS_PER_TARGET);
> >  
> >      return reg;
> >  }
> 
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.