[PATCH] ARM: Fix find_next_zero_bit and related assembly

[PATCH] ARM: Fix find_next_zero_bit and related assembly

[Nicolas Pitre]

On Thu, 11 Nov 2010, James Jones wrote:

> The find_next_bit, find_first_bit, find_next_zero_bit
> and find_first_zero_bit functions were not properly
> clamping to the maxbit argument at the bit level. They
> were instead only checking maxbit at the byte level.
> To fix this, add a compare and a conditional move
> instruction to the end of the common bit-within-the-
> byte code used by all the functions and be sure not to
> clobber the maxbit argument before it is used.
> 
> Signed-off-by: James Jones <jajones@nvidia.com>
> Tested-by: Stephen Warren <swarren@nvidia.com>

Reviewed-by: Nicolas Pitre <nicolas.pitre@linaro.org>

Please send to RMK's patch system.



> ---
>  arch/arm/lib/findbit.S |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/lib/findbit.S b/arch/arm/lib/findbit.S
> index 1e4cbd4..64f6bc1 100644
> --- a/arch/arm/lib/findbit.S
> +++ b/arch/arm/lib/findbit.S
> @@ -174,8 +174,8 @@ ENDPROC(_find_next_bit_be)
>   */
>  .L_found:
>  #if __LINUX_ARM_ARCH__ >= 5
> -		rsb	r1, r3, #0
> -		and	r3, r3, r1
> +		rsb	r0, r3, #0
> +		and	r3, r3, r0
>  		clz	r3, r3
>  		rsb	r3, r3, #31
>  		add	r0, r2, r3
> @@ -190,5 +190,7 @@ ENDPROC(_find_next_bit_be)
>  		addeq	r2, r2, #1
>  		mov	r0, r2
>  #endif
> +		cmp	r1, r0			@ Clamp to maxbit
> +		movlo	r0, r1
>  		mov	pc, lr
>  
> -- 
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

[PATCH] ARM: Fix find_next_zero_bit and related assembly

[James Jones]

On Tuesday 23 November 2010 13:26:47 Nicolas Pitre wrote:
> On Thu, 11 Nov 2010, James Jones wrote:
> > The find_next_bit, find_first_bit, find_next_zero_bit
> > and find_first_zero_bit functions were not properly
> > clamping to the maxbit argument at the bit level. They
> > were instead only checking maxbit at the byte level.
> > To fix this, add a compare and a conditional move
> > instruction to the end of the common bit-within-the-
> > byte code used by all the functions and be sure not to
> > clobber the maxbit argument before it is used.
> > 
> > Signed-off-by: James Jones <jajones@nvidia.com>
> > Tested-by: Stephen Warren <swarren@nvidia.com>
> 
> Reviewed-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> 
> Please send to RMK's patch system.

Thanks for the review.  It's already in the patch system, but I updated the 
entry to include your reviewed-by line.

-James

> > ---
> > 
> >  arch/arm/lib/findbit.S |    6 ++++--
> >  1 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/lib/findbit.S b/arch/arm/lib/findbit.S
> > index 1e4cbd4..64f6bc1 100644
> > --- a/arch/arm/lib/findbit.S
> > +++ b/arch/arm/lib/findbit.S
> > @@ -174,8 +174,8 @@ ENDPROC(_find_next_bit_be)
> > 
> >   */
> >  
> >  .L_found:
> >  #if __LINUX_ARM_ARCH__ >= 5
> > 
> > -		rsb	r1, r3, #0
> > -		and	r3, r3, r1
> > +		rsb	r0, r3, #0
> > +		and	r3, r3, r0
> > 
> >  		clz	r3, r3
> >  		rsb	r3, r3, #31
> >  		add	r0, r2, r3
> > 
> > @@ -190,5 +190,7 @@ ENDPROC(_find_next_bit_be)
> > 
> >  		addeq	r2, r2, #1
> >  		mov	r0, r2
> >  
> >  #endif
> > 
> > +		cmp	r1, r0			@ Clamp to maxbit
> > +		movlo	r0, r1
> > 
> >  		mov	pc, lr

[PATCH] ARM: Fix find_next_zero_bit and related assembly

[Stephen Boyd]

On 11/23/2010 03:28 PM, James Jones wrote:
> On Tuesday 23 November 2010 13:26:47 Nicolas Pitre wrote:
>> On Thu, 11 Nov 2010, James Jones wrote:
>>> The find_next_bit, find_first_bit, find_next_zero_bit
>>> and find_first_zero_bit functions were not properly
>>> clamping to the maxbit argument at the bit level. They
>>> were instead only checking maxbit at the byte level.
>>> To fix this, add a compare and a conditional move
>>> instruction to the end of the common bit-within-the-
>>> byte code used by all the functions and be sure not to
>>> clobber the maxbit argument before it is used.
>>>
>>> Signed-off-by: James Jones <jajones@nvidia.com>
>>> Tested-by: Stephen Warren <swarren@nvidia.com>
>> Reviewed-by: Nicolas Pitre <nicolas.pitre@linaro.org>
>>
>> Please send to RMK's patch system.
> Thanks for the review.  It's already in the patch system, but I updated the 
> entry to include your reviewed-by line.

Should this be sent to the stable tree too?

[PATCH] ARM: Fix find_next_zero_bit and related assembly

[Nicolas Pitre]

On Wed, 24 Nov 2010, Stephen Boyd wrote:

> On 11/23/2010 03:28 PM, James Jones wrote:
> > On Tuesday 23 November 2010 13:26:47 Nicolas Pitre wrote:
> >> On Thu, 11 Nov 2010, James Jones wrote:
> >>> The find_next_bit, find_first_bit, find_next_zero_bit
> >>> and find_first_zero_bit functions were not properly
> >>> clamping to the maxbit argument at the bit level. They
> >>> were instead only checking maxbit at the byte level.
> >>> To fix this, add a compare and a conditional move
> >>> instruction to the end of the common bit-within-the-
> >>> byte code used by all the functions and be sure not to
> >>> clobber the maxbit argument before it is used.
> >>>
> >>> Signed-off-by: James Jones <jajones@nvidia.com>
> >>> Tested-by: Stephen Warren <swarren@nvidia.com>
> >> Reviewed-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> >>
> >> Please send to RMK's patch system.
> > Thanks for the review.  It's already in the patch system, but I updated the 
> > entry to include your reviewed-by line.
> 
> Should this be sent to the stable tree too?

It could, yes.  This is hardly an urgent fix though, as the bug has been 
there virtually forever.


Nicolas

[PATCH] ARM: Fix find_next_zero_bit and related assembly

[James Jones]

On Wednesday 24 November 2010 11:00:12 am Nicolas Pitre wrote:
> On Wed, 24 Nov 2010, Stephen Boyd wrote:
> > On 11/23/2010 03:28 PM, James Jones wrote:
> > > On Tuesday 23 November 2010 13:26:47 Nicolas Pitre wrote:
> > >> On Thu, 11 Nov 2010, James Jones wrote:
> > >>> The find_next_bit, find_first_bit, find_next_zero_bit
> > >>> and find_first_zero_bit functions were not properly
> > >>> clamping to the maxbit argument at the bit level. They
> > >>> were instead only checking maxbit at the byte level.
> > >>> To fix this, add a compare and a conditional move
> > >>> instruction to the end of the common bit-within-the-
> > >>> byte code used by all the functions and be sure not to
> > >>> clobber the maxbit argument before it is used.
> > >>> 
> > >>> Signed-off-by: James Jones <jajones@nvidia.com>
> > >>> Tested-by: Stephen Warren <swarren@nvidia.com>
> > >> 
> > >> Reviewed-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> > >> 
> > >> Please send to RMK's patch system.
> > > 
> > > Thanks for the review.  It's already in the patch system, but I updated
> > > the entry to include your reviewed-by line.
> > 
> > Should this be sent to the stable tree too?
> 
> It could, yes.  This is hardly an urgent fix though, as the bug has been
> there virtually forever.
> 
> 
> Nicolas

While ancient, it does cause per-cpu allocations to fail in some situations, 
which generally causes panics.

Thanks,
-James

[PATCH] ARM: Fix find_next_zero_bit and related assembly

[Nicolas Pitre]

On Wed, 24 Nov 2010, James Jones wrote:

> On Wednesday 24 November 2010 11:00:12 am Nicolas Pitre wrote:
> > On Wed, 24 Nov 2010, Stephen Boyd wrote:
> > > On 11/23/2010 03:28 PM, James Jones wrote:
> > > > On Tuesday 23 November 2010 13:26:47 Nicolas Pitre wrote:
> > > >> On Thu, 11 Nov 2010, James Jones wrote:
> > > >>> The find_next_bit, find_first_bit, find_next_zero_bit
> > > >>> and find_first_zero_bit functions were not properly
> > > >>> clamping to the maxbit argument at the bit level. They
> > > >>> were instead only checking maxbit at the byte level.
> > > >>> To fix this, add a compare and a conditional move
> > > >>> instruction to the end of the common bit-within-the-
> > > >>> byte code used by all the functions and be sure not to
> > > >>> clobber the maxbit argument before it is used.
> > > >>> 
> > > >>> Signed-off-by: James Jones <jajones@nvidia.com>
> > > >>> Tested-by: Stephen Warren <swarren@nvidia.com>
> > > >> 
> > > >> Reviewed-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> > > >> 
> > > >> Please send to RMK's patch system.
> > > > 
> > > > Thanks for the review.  It's already in the patch system, but I updated
> > > > the entry to include your reviewed-by line.
> > > 
> > > Should this be sent to the stable tree too?
> > 
> > It could, yes.  This is hardly an urgent fix though, as the bug has been
> > there virtually forever.
> > 
> > 
> > Nicolas
> 
> While ancient, it does cause per-cpu allocations to fail in some situations, 
> which generally causes panics.

That would be a good justification for the stable tree then.


Nicolas

[PATCH] ARM: Fix find_next_zero_bit and related assembly

[Russell King - ARM Linux]

On Wed, Nov 24, 2010 at 02:13:05PM -0500, Nicolas Pitre wrote:
> On Wed, 24 Nov 2010, James Jones wrote:
> 
> > On Wednesday 24 November 2010 11:00:12 am Nicolas Pitre wrote:
> > > On Wed, 24 Nov 2010, Stephen Boyd wrote:
> > > > On 11/23/2010 03:28 PM, James Jones wrote:
> > > > > On Tuesday 23 November 2010 13:26:47 Nicolas Pitre wrote:
> > > > >> On Thu, 11 Nov 2010, James Jones wrote:
> > > > >>> The find_next_bit, find_first_bit, find_next_zero_bit
> > > > >>> and find_first_zero_bit functions were not properly
> > > > >>> clamping to the maxbit argument at the bit level. They
> > > > >>> were instead only checking maxbit at the byte level.
> > > > >>> To fix this, add a compare and a conditional move
> > > > >>> instruction to the end of the common bit-within-the-
> > > > >>> byte code used by all the functions and be sure not to
> > > > >>> clobber the maxbit argument before it is used.
> > > > >>> 
> > > > >>> Signed-off-by: James Jones <jajones@nvidia.com>
> > > > >>> Tested-by: Stephen Warren <swarren@nvidia.com>
> > > > >> 
> > > > >> Reviewed-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> > > > >> 
> > > > >> Please send to RMK's patch system.
> > > > > 
> > > > > Thanks for the review.  It's already in the patch system, but I updated
> > > > > the entry to include your reviewed-by line.
> > > > 
> > > > Should this be sent to the stable tree too?
> > > 
> > > It could, yes.  This is hardly an urgent fix though, as the bug has been
> > > there virtually forever.
> > > 
> > > 
> > > Nicolas
> > 
> > While ancient, it does cause per-cpu allocations to fail in some situations, 
> > which generally causes panics.
> 
> That would be a good justification for the stable tree then.

And the best way to do that is _not_ to send it to stable at kernel.org,
but when you submit the patch to the patch system, add a line in
the sign-off area:

CC: <stable@kernel.org>

and when it goes into mainline (which is a requirement for stable
patches) the stable team will automatically pick it up - without anyone
needing to do any additional work.

[PATCH] ARM: Fix find_next_zero_bit and related assembly

[James Jones]

The find_next_bit, find_first_bit, find_next_zero_bit
and find_first_zero_bit functions were not properly
clamping to the maxbit argument at the bit level. They
were instead only checking maxbit at the byte level.
To fix this, add a compare and a conditional move
instruction to the end of the common bit-within-the-
byte code used by all the functions and be sure not to
clobber the maxbit argument before it is used.

Signed-off-by: James Jones <jajones@nvidia.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
---
 arch/arm/lib/findbit.S |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/lib/findbit.S b/arch/arm/lib/findbit.S
index 1e4cbd4..64f6bc1 100644
--- a/arch/arm/lib/findbit.S
+++ b/arch/arm/lib/findbit.S
@@ -174,8 +174,8 @@ ENDPROC(_find_next_bit_be)
  */
 .L_found:
 #if __LINUX_ARM_ARCH__ >= 5
-		rsb	r1, r3, #0
-		and	r3, r3, r1
+		rsb	r0, r3, #0
+		and	r3, r3, r0
 		clz	r3, r3
 		rsb	r3, r3, #31
 		add	r0, r2, r3
@@ -190,5 +190,7 @@ ENDPROC(_find_next_bit_be)
 		addeq	r2, r2, #1
 		mov	r0, r2
 #endif
+		cmp	r1, r0			@ Clamp to maxbit
+		movlo	r0, r1
 		mov	pc, lr
 
-- 
1.7.1