Bug 29197 - __strncpy_power9() uses uninitialised register vs18 value for filling after \0
Summary: __strncpy_power9() uses uninitialised register vs18 value for filling after \0
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: string (show other bugs)
Version: 2.35
: P2 normal
Target Milestone: 2.36
Assignee: Matheus Castanho
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-05-29 19:39 UTC by Zdenek Sojka
Modified: 2022-06-07 20:00 UTC (History)
5 users (show)

See Also:
Host:
Target: powerpc64le-unknown-linux-gnu
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Zdenek Sojka 2022-05-29 19:39:12 UTC
Hello,

originally reported as a gcc bug at https://gcc.gnu.org/PR105744 , but it was analysed by Kewen Lin to be an issue in the glibc implementation.

The following program:
$ cat test_strncpy.c
#include <string.h>
#include <stdlib.h>

#define N 3
char a[N];
char c[N];

int
main (void)
{
  __asm__ volatile ("xxspltib 18, 0xf":::"vs18");
  strncpy (c, a, N);
  for (unsigned i = 0; i < N; i++)
    if (c[i])
      abort ();
  return 0;
}
$ powerpc64le-unknown-linux-gnu-gcc test_strncpy.c -static
$ qemu-ppc64le -- ./a.out 
qemu: uncaught target signal 6 (Aborted) - core dumped
Aborted

fails, with the value of c[] being:
(gdb) p c
$1 = "\000\015\015"


Quoting Kewen Lin:
---------------------
In https://sourceware.org/git/?p=glibc.git;a=blob_plain;f=sysdeps/powerpc/powerpc64/le/power9/strncpy.S

	lbz	r0,0(r4)
	stb	r0,0(r3)
	addi	r11,r3,1
	addi	r5,r5,-1
	vspltisb v18,0		/* Zeroes in v18  */

...

L(zero_padding_end):
	sldi	r10,r5,56	/* stxvl wants size in top 8 bits  */
	stxvl	v18,r11,r10	/* Partial store  */
	blr


The code at label "zero_padding_end" is supposed to use v18, but the stxvl will take the 18 as vsx No. instead of vr No, so it ends up to use the wrong register vs18 instead of v18 for the store.
---------------------

Disassembling the object code shows the registers used (v18 / vs18):

...
   0x0000000010022f10 <+16>:      lbz     r0,0(r4)
   0x0000000010022f14 <+20>:      stb     r0,0(r3)
   0x0000000010022f18 <+24>:      addi    r11,r3,1
   0x0000000010022f1c <+28>:      addi    r5,r5,-1
   0x0000000010022f20 <+32>:      vspltisb v18,0
...
   0x000000001002319c <+668>:     rldicr  r10,r5,56,7
   0x00000000100231a0 <+672>:     stxvl   vs18,r11,r10
   0x00000000100231a4 <+676>:     blr
Comment 1 Kewen.Lin 2022-06-01 08:04:17 UTC
Thanks for filing this, Zdenek!

By referring to the existing code, one fix seems to be:

diff --git a/sysdeps/powerpc/powerpc64/le/power9/strncpy.S b/sysdeps/powerpc/powerpc64/le/power9/strncpy.S
index ae23161316..deb94671cc 100644
--- a/sysdeps/powerpc/powerpc64/le/power9/strncpy.S
+++ b/sysdeps/powerpc/powerpc64/le/power9/strncpy.S
@@ -352,7 +352,7 @@ L(zero_padding_loop):
 	cmpldi	cr6,r5,16	/* Check if length was reached.  */
 	ble	cr6,L(zero_padding_end)

-	stxv	v18,0(r11)
+	stxv	32+v18,0(r11)
 	addi	r11,r11,16
 	addi	r5,r5,-16

@@ -360,7 +360,7 @@ L(zero_padding_loop):

 L(zero_padding_end):
 	sldi	r10,r5,56	/* stxvl wants size in top 8 bits  */
-	stxvl	v18,r11,r10	/* Partial store  */
+	stxvl	32+v18,r11,r10	/* Partial store  */
 	blr

 	.align	4
Comment 2 Matheus Castanho 2022-06-06 18:57:13 UTC
I was able to reproduce the error and I confirm Kewen.Lin's patch fixes the issue. Besides the reproducer listed above, all tests pass.

Kewen.Lin, do you want to submit the fix to libc-alpha yourself or would you prefer if someone else did this instead?
Comment 3 Kewen.Lin 2022-06-07 03:04:17 UTC
(In reply to Matheus Castanho from comment #2)
> I was able to reproduce the error and I confirm Kewen.Lin's patch fixes the
> issue. Besides the reproducer listed above, all tests pass.
> 
> Kewen.Lin, do you want to submit the fix to libc-alpha yourself or would you
> prefer if someone else did this instead?

Hi Matheus,

Thanks for testing and confirming! I'm not familiar with the libc-alpha contribution flow and meant to post some more findings, if you or anyone else wants to take over it, feel free to do that. Thanks in advance! :)
Comment 4 Matheus Castanho 2022-06-07 14:32:01 UTC
Ok then. I ran some more tests and submitted it to libc-alpha: https://sourceware.org/pipermail/libc-alpha/2022-June/139494.html

Thank you for your help.
Comment 5 Matheus Castanho 2022-06-07 19:03:13 UTC
The fix has been merged as commit 0218463dd8265ed937622f88ac68c7d984fe0cfc and backported to all affected releases: 2.33, 2.34 and 2.35.