[PATCH] power8: fix read past array bounds in strncasecmp (bug 34683)

[Adhemerval Zanella Netto]

On 29/09/26 10:58, Andreas Schwab wrote:
> Move the load of the characters to the top of the loops, so that they
> occur after the bounds check for the strncasecmp implementation.  Amend
> the page test for strncasecmp to also test with non-null-terminated arrays
> that trigger this bug.

We have two slight different patches that fix the same issue [1]. Which should
we use?

[1] https://patchwork.sourceware.org/project/glibc/patch/20260929112822.2099581-1-smonga@linux.ibm.com/

> ---
>  string/test-strncasecmp.c                     | 16 ++++++++++++++++
>  sysdeps/powerpc/powerpc64/power8/strcasecmp.S | 14 ++++++--------
>  2 files changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/string/test-strncasecmp.c b/string/test-strncasecmp.c
> index 6b00113e66..3fced4202b 100644
> --- a/string/test-strncasecmp.c
> +++ b/string/test-strncasecmp.c
> @@ -199,6 +199,22 @@ do_page_tests (void)
>  			  exp_result);
>  	}
>      }
> +
> +  /* Like strncmp, the arrays passed to strncasecmp are not required to be
> +     null-terminated.  Check that the implementations do not read beyond
> +     the end of the arrays.  */
> +
> +  s1[maxoffset - 1] = 'a';
> +  s2[maxoffset - 1] = 'a';
> +  for (size_t off1 = 0; off1 < maxoffset; off1++)
> +    {
> +      for (size_t off2 = 0; off2 < maxoffset; off2++)
> +	{
> +	  size_t off_max = off1 < off2 ? off2 : off1;
> +	  FOR_EACH_IMPL (impl, 0)
> +	    check_result (impl, s1 + off1, s2 + off2, maxoffset - off_max, 0);
> +	}
> +    }
>  }
>  
>  static void
> diff --git a/sysdeps/powerpc/powerpc64/power8/strcasecmp.S b/sysdeps/powerpc/powerpc64/power8/strcasecmp.S
> index 3c45e9ea50..22db297855 100644
> --- a/sysdeps/powerpc/powerpc64/power8/strcasecmp.S
> +++ b/sysdeps/powerpc/powerpc64/power8/strcasecmp.S
> @@ -355,8 +355,6 @@ L(bytebybyte):
>  	offset and string descriptors are only updated in the end
>  	of loop unrolling. */
>  	ld	rLOC, LOCALE_CTYPE_TOLOWER(rLOC)
> -	lbz	rCHAR1, 0(rSTR1)	/* Load char from s1 */
> -	lbz	rCHAR2, 0(rSTR2)	/* Load char from s2 */
>  #ifdef USE_AS_STRNCASECMP
>  	rldicl	rTMP, r5, 62, 2
>  	cmpdi	cr7, rTMP, 0
> @@ -364,6 +362,8 @@ L(bytebybyte):
>  	mtctr	rTMP
>  #endif
>  L(loop):
> +	lbz	rCHAR1, 0(rSTR1)	/* Load char from s1 */
> +	lbz	rCHAR2, 0(rSTR2)	/* Load char from s2 */
>  	cmpdi	rCHAR1, 0		/* *s1 == '\0' ? */
>  	sldi	rADDR1, rCHAR1, 2	/* Calculate address for tolower(*s1) */
>  	sldi	rADDR2, rCHAR2, 2	/* Calculate address for tolower(*s2) */
> @@ -404,13 +404,11 @@ L(loop):
>  	lwzx	rLWR2, rLOC, rADDR2
>  	cmpw	cr1, rLWR1, rLWR2
>  	crorc	4*cr1+eq,eq,4*cr1+eq
> -	beq     cr1, L(done)
> -	lbz	rCHAR1, 0(rSTR1)	/* Load char from s1 */
> -	lbz	rCHAR2, 0(rSTR2)	/* Load char from s2 */
>  #ifdef USE_AS_STRNCASECMP
> +	beq     cr1, L(done)
>  	bdnz	L(loop)
>  #else
> -	b	L(loop)
> +	bne     cr1, L(loop)
>  #endif
>  #ifdef USE_AS_STRNCASECMP
>  L(lessthan4):
> @@ -419,6 +417,8 @@ L(lessthan4):
>  	beq	cr7, L(retnull)
>  	mtctr	r5
>  L(loop1):
> +	lbz	rCHAR1, 0(rSTR1)
> +	lbz	rCHAR2, 0(rSTR2)
>  	cmpdi	rCHAR1, 0
>  	sldi	rADDR1, rCHAR1, 2
>  	sldi	rADDR2, rCHAR2, 2
> @@ -429,8 +429,6 @@ L(loop1):
>  	beq	cr1, L(done)
>  	addi	rSTR1, rSTR1, 1
>  	addi	rSTR2, rSTR2, 1
> -	lbz	rCHAR1, 0(rSTR1)
> -	lbz	rCHAR2, 0(rSTR2)
>  	bdnz	L(loop1)
>  #endif
>  L(done):