Re: [Xen-devel] [PATCH] xen: arm: correct use of find_next_bit

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2014-01-24 at 14:36 +0000, Julien Grall wrote:
> On 01/24/2014 02:23 PM, Ian Campbell wrote:
> > find_next_bit takes a "const unsigned long *" but forcing a cast of an
> > "uint32_t *" throws away the alignment constraints and ends up causing an
> > alignment fault on arm64 if the input happened to be 4 but not 8 byte 
> > aligned.
> > 
> > Instead of casting use a temporary variable of the right type.
> > 
> > I've had a look around for similar constructs and the only thing I found was
> > maintenance_interrupt which cases a uint64_t down to an unsigned long, which
> > although perhaps not best advised is safe I think.
> > 
> > This was observed with the AArch64 Linaro toolchain 2013.12 but I think that
> > is just coincidental due to subtle changes to the stack layout etc.
> > 
> > Reported-by: Fu Wei <fu.wei@xxxxxxxxxx>
> > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> 
> Good catch! Do you plan to apply this patch for Xen 4.4?

Yes, I think it is a suitable bug fix.

> 
> Acked-by: Julien Grall <julien.grall@xxxxxxxxxx>
> 
> > ---
> >  xen/arch/arm/vgic.c |    6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> > index 90e9707..553411d 100644
> > --- a/xen/arch/arm/vgic.c
> > +++ b/xen/arch/arm/vgic.c
> > @@ -362,11 +362,12 @@ read_as_zero:
> >  
> >  static void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
> >  {
> > +    const unsigned long mask = r;
> >      struct pending_irq *p;
> >      unsigned int irq;
> >      int i = 0;
> >  
> > -    while ( (i = find_next_bit((const long unsigned int *) &r, 32, i)) < 
> > 32 ) {
> > +    while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
> >          irq = i + (32 * n);
> >          p = irq_to_pending(v, irq);
> >          clear_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
> > @@ -379,11 +380,12 @@ static void vgic_disable_irqs(struct vcpu *v, 
> > uint32_t r, int n)
> >  
> >  static void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
> >  {
> > +    const unsigned long mask = r;
> >      struct pending_irq *p;
> >      unsigned int irq;
> >      int i = 0;
> >  
> > -    while ( (i = find_next_bit((const long unsigned int *) &r, 32, i)) < 
> > 32 ) {
> > +    while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
> >          irq = i + (32 * n);
> >          p = irq_to_pending(v, irq);
> >          set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
> > 
> 
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel