[Linux-ia64] IA64 strncpy in 2.2.4-30 - bug and patch

[Linux-ia64] IA64 strncpy in 2.2.4-30 - bug and patch

[Reese Faucette]

[-- Attachment #1.1: Type: text/plain, Size: 566 bytes --]

Hi,
I opened a bug with RedHat about a problem with strncpy() in glibc-2.2.4-30 on IA64, see https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=76952, but I've been told you folks are really the ones to talk to.

The bug report contains a testcase illustrating the problem, and also a patch.  In short, the recovery code is busted, even after the patch from David Mosberger.  Attached is a copy of the patch from the bug report.

I have a testcase which exercizes both the recovery3 and recovery4 paths if you're interested.  
Regards,
-reese faucette



[-- Attachment #1.2: Type: text/html, Size: 1381 bytes --]

[-- Attachment #2: strncpy.patch --]
[-- Type: application/octet-stream, Size: 437 bytes --]

--- strncpy.S	Thu Oct 31 15:19:46 2002
+++ new/strncpy.S	Thu Oct 31 17:11:34 2002
@@ -214,11 +214,11 @@
 	ld8	r[0] = [tmp]
 	br.cond.sptk .back2
 .recovery3:
-	add	tmp = -MEMLAT * 8, src ;;
+	add	tmp = -(MEMLAT+1) * 8, src ;;
 	ld8	r[MEMLAT] = [tmp]
 	br.cond.sptk .back3
 .recovery4:
-	add	tmp = -(MEMLAT - 1) * 8, src ;;
+	add	tmp = -(MEMLAT) * 8, src ;;
 	ld8	r[MEMLAT - 1] = [tmp]
 	br.cond.sptk .back4
 END(strncpy)

RE: [Linux-ia64] IA64 strncpy in 2.2.4-30 - bug and patch

[Chen, Kenneth W]

I'm not able to reproduce the bug with your test case ...

-----Original Message-----
From: Reese Faucette [mailto:reese@myri.com]
Sent: Tuesday, November 05, 2002 4:39 PM
To: bug-glibc@gnu.org; linux-ia64@linuxia64.org
Cc: Akyil, Levent; susan@myri.com
Subject: [Linux-ia64] IA64 strncpy in 2.2.4-30 - bug and patch


Hi,
I opened a bug with RedHat about a problem with strncpy() in glibc-2.2.4-30 on IA64, see https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=76952, but I've been told you folks are really the ones to talk to.
 
The bug report contains a testcase illustrating the problem, and also a patch.  In short, the recovery code is busted, even after the patch from David Mosberger.  Attached is a copy of the patch from the bug report.
 
I have a testcase which exercizes both the recovery3 and recovery4 paths if you're interested.  
Regards,
-reese faucette
 
 

RE: [Linux-ia64] IA64 strncpy in 2.2.4-30 - bug and patch

[Chen, Kenneth W]

[-- Attachment #1: Type: text/plain, Size: 1531 bytes --]

Here is a patch that fixes bugs in strncpy function included in glibc-2.2.4.  We found there are more bugs during the investigation.  Please pound on this patch and let us know any issues.

- Ken Chen


-----Original Message-----
From: Chen, Kenneth W 
Sent: Tuesday, November 05, 2002 4:44 PM
To: Reese Faucette; bug-glibc@gnu.org; linux-ia64@linuxia64.org
Cc: Akyil, Levent; susan@myri.com
Subject: RE: [Linux-ia64] IA64 strncpy in 2.2.4-30 - bug and patch


I'm not able to reproduce the bug with your test case ...

-----Original Message-----
From: Reese Faucette [mailto:reese@myri.com]
Sent: Tuesday, November 05, 2002 4:39 PM
To: bug-glibc@gnu.org; linux-ia64@linuxia64.org
Cc: Akyil, Levent; susan@myri.com
Subject: [Linux-ia64] IA64 strncpy in 2.2.4-30 - bug and patch


Hi,
I opened a bug with RedHat about a problem with strncpy() in glibc-2.2.4-30 on IA64, see https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=76952, but I've been told you folks are really the ones to talk to.
 
The bug report contains a testcase illustrating the problem, and also a patch.  In short, the recovery code is busted, even after the patch from David Mosberger.  Attached is a copy of the patch from the bug report.
 
I have a testcase which exercizes both the recovery3 and recovery4 paths if you're interested.  
Regards,
-reese faucette
 
 


_______________________________________________
Linux-IA64 mailing list
Linux-IA64@linuxia64.org
http://lists.linuxia64.org/lists/listinfo/linux-ia64

[-- Attachment #2: strncpy.fix.patch --]
[-- Type: application/octet-stream, Size: 1122 bytes --]

--- strncpy.S.orig	Wed Nov  6 17:16:31 2002
+++ strncpy.S	Wed Nov  6 18:47:25 2002
@@ -48,6 +48,7 @@
 #define	sh1		r29
 #define loopcnt		r30
 #define	value		r31
+#define tmp2		r14
 
 ENTRY(strncpy)
 	.prologue
@@ -62,6 +63,7 @@
 	mov	saved_pr = pr           // save the predicate registers
 	.save ar.lc, saved_lc
 	mov 	saved_lc = ar.lc	// save the loop counter
+	mov	ar.ec = 0
 	.body
 	cmp.geu p6, p5 = 24, in2
 (p6)	br.cond.spnt .short_len
@@ -209,15 +211,21 @@
 	mov	pr = saved_pr, -1	// restore the predicate registers
 	br.ret.sptk.many b0
 .recovery2:
+	sub	tmp2=len,thresh
+	;;
+	cmp.eq	p8,p9=-8, tmp2
 	add	tmp = -8, asrc ;;
-	ld8	r[0] = [tmp]
+(p8)	mov	r[0] = r0
+(p9)	ld8	r[0] = [tmp]
 	br.cond.sptk .back2
 .recovery3:
-	add	tmp = -MEMLAT * 8, src ;;
+	add	tmp = -(MEMLAT+1) * 8, src ;;
 	ld8	r[MEMLAT] = [tmp]
 	br.cond.sptk .back3
 .recovery4:
-	add	tmp = -(MEMLAT - 1) * 8, src ;;
-	ld8	r[MEMLAT - 1] = [tmp]
+	cmp.eq	p8,p9=0, len
+	add	tmp = -(MEMLAT) * 8, src ;;
+(p8)	mov	r[MEMLAT - 1] = r0
+(p9)	ld8	r[MEMLAT - 1] = [tmp]
 	br.cond.sptk .back4
 END(strncpy)

RE: [Linux-ia64] IA64 strncpy in 2.2.4-30 - bug and patch

[Chen, Kenneth W]

[-- Attachment #1: Type: text/plain, Size: 2045 bytes --]

We found more bugs in the code, and here is an updated patch to fix them.
Note this patch supercedes the one I sent yesterday.

Also attached is the test program we used to verify the fix.

- Ken Chen


-----Original Message-----
From: Chen, Kenneth W 
Sent: Wednesday, November 06, 2002 6:57 PM
To: Chen, Kenneth W; Reese Faucette; bug-glibc@gnu.org;
linux-ia64@linuxia64.org
Cc: Akyil, Levent; susan@myri.com; Mallick, Asit K
Subject: RE: [Linux-ia64] IA64 strncpy in 2.2.4-30 - bug and patch


Here is a patch that fixes bugs in strncpy function included in glibc-2.2.4.  We found there are more bugs during the investigation.  Please pound on this patch and let us know any issues.

- Ken Chen


-----Original Message-----
From: Chen, Kenneth W 
Sent: Tuesday, November 05, 2002 4:44 PM
To: Reese Faucette; bug-glibc@gnu.org; linux-ia64@linuxia64.org
Cc: Akyil, Levent; susan@myri.com
Subject: RE: [Linux-ia64] IA64 strncpy in 2.2.4-30 - bug and patch


I'm not able to reproduce the bug with your test case ...

-----Original Message-----
From: Reese Faucette [mailto:reese@myri.com]
Sent: Tuesday, November 05, 2002 4:39 PM
To: bug-glibc@gnu.org; linux-ia64@linuxia64.org
Cc: Akyil, Levent; susan@myri.com
Subject: [Linux-ia64] IA64 strncpy in 2.2.4-30 - bug and patch


Hi,
I opened a bug with RedHat about a problem with strncpy() in glibc-2.2.4-30 on IA64, see https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=76952, but I've been told you folks are really the ones to talk to.
 
The bug report contains a testcase illustrating the problem, and also a patch.  In short, the recovery code is busted, even after the patch from David Mosberger.  Attached is a copy of the patch from the bug report.
 
I have a testcase which exercizes both the recovery3 and recovery4 paths if you're interested.  
Regards,
-reese faucette
 
 


_______________________________________________
Linux-IA64 mailing list
Linux-IA64@linuxia64.org
http://lists.linuxia64.org/lists/listinfo/linux-ia64

[-- Attachment #2: strncpy2.fix.patch --]
[-- Type: application/octet-stream, Size: 1122 bytes --]

--- strncpy.S.orig	Wed Nov  6 17:16:31 2002
+++ strncpy.S	Wed Nov  6 18:47:25 2002
@@ -48,6 +48,7 @@
 #define	sh1		r29
 #define loopcnt		r30
 #define	value		r31
+#define tmp2		r14
 
 ENTRY(strncpy)
 	.prologue
@@ -62,6 +63,7 @@
 	mov	saved_pr = pr           // save the predicate registers
 	.save ar.lc, saved_lc
 	mov 	saved_lc = ar.lc	// save the loop counter
+	mov	ar.ec = 0
 	.body
 	cmp.geu p6, p5 = 24, in2
 (p6)	br.cond.spnt .short_len
@@ -209,15 +211,21 @@
 	mov	pr = saved_pr, -1	// restore the predicate registers
 	br.ret.sptk.many b0
 .recovery2:
+	sub	tmp2=len,thresh
+	;;
+	cmp.ge	p8,p9=-8, tmp2
 	add	tmp = -8, asrc ;;
-	ld8	r[0] = [tmp]
+(p8)	mov	r[0] = r0
+(p9)	ld8	r[0] = [tmp]
 	br.cond.sptk .back2
 .recovery3:
-	add	tmp = -MEMLAT * 8, src ;;
+	add	tmp = -(MEMLAT+1) * 8, src ;;
 	ld8	r[MEMLAT] = [tmp]
 	br.cond.sptk .back3
 .recovery4:
-	add	tmp = -(MEMLAT - 1) * 8, src ;;
-	ld8	r[MEMLAT - 1] = [tmp]
+	cmp.eq	p8,p9=0, len
+	add	tmp = -(MEMLAT) * 8, src ;;
+(p8)	mov	r[MEMLAT - 1] = r0
+(p9)	ld8	r[MEMLAT - 1] = [tmp]
 	br.cond.sptk .back4
 END(strncpy)

[-- Attachment #3: bug.c --]
[-- Type: application/octet-stream, Size: 1341 bytes --]

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <sys/shm.h>

#define MCPYSIZE 2048

extern char *my_strncpy(char *dest, const char *src, size_t n);

int main()
{
  long buffer[MCPYSIZE];

  int i, j, k, x;
  char *src, *dst;
  size_t len;
  int shmid;
  char* shmaddr;


  shmid = shmget(1, 16384, IPC_CREAT|SHM_R|SHM_W);
  shmaddr = (char*) shmat(shmid, NULL, SHM_RND);

  for (i=0; i<16384; i++)
    shmaddr[i] = i;

  dst = (char*) buffer;

  for (k=1; k<1000; k+=1)
    for (i=0; i<8; i++)
      for (j=0; j<8; j++) {

	src = (char*) shmaddr + 16384 - k - i;
	dst = (char*) buffer + j;
	len  = k;

//	printf("dst %p src %p n %4d\n", dst, src, len);
	my_strncpy(dst, src, len);

	for (x=0; x<len; x++) {
		if (src[x] == 0) break;
		if ( dst[x] != src[x] ) {
			unsigned long * l_src = (unsigned long*) ((unsigned long) &src[x] & -8);
			unsigned long * l_dst = (unsigned long*) ((unsigned long) &dst[x] & -8);

			printf("error\t");
			printf("dst %p src %p n %4d\n", dst, src, len);

			printf("%d: %x %x\n", x, dst[x], src[x]);

			printf("%d: %16x %16x\n", x, *l_dst, *l_src);
			exit(0);
//			break;
		}
	}

//	if (strcmp(dst, src, len) != 0)
//	  printf("bug found\n");

      } // for (j=0 ...

  shmdt(shmaddr);
  shmctl(shmid, IPC_RMID, NULL);
}

[-- Attachment #4: bug2.c --]
[-- Type: application/octet-stream, Size: 1468 bytes --]

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <sys/shm.h>
#include <sys/mman.h>
#include <unistd.h>
#include <fcntl.h>

#define MCPYSIZE 2048

extern char *my_strncpy(char *dest, const char *src, size_t n);

int main()
{
  long buffer[MCPYSIZE];

  int i, j, k, l, x;
  char *src, *dst;
  size_t len;
  int shmid;
  char* shmaddr;
  int fd;

/*
  shmid = shmget(1, 16384, IPC_CREAT|SHM_R|SHM_W);
  shmaddr = (char*) shmat(shmid, NULL, SHM_RND);
*/
  fd = open("data", O_RDONLY);

  dst = (char*) buffer;

  for (k=0; k<10000; k+=1)
   for (l=0; l<10; l++)
    for (i=0; i<8; i++)
      for (j=0; j<8; j++) {

  shmaddr = (char*) mmap(NULL, 16384*2, PROT_READ, MAP_SHARED, fd, 0);
  if (shmaddr == NULL) perror("zero address");

	src = (char*) shmaddr + 16384 - k - i;
	dst = (char*) buffer + j;
	len  = k+l;

//	printf("dst %p src %p n %4d\n", dst, src, len);
	my_strncpy(dst, src, len);

	for (x=0; x<len; x++) {
		if (src[x] == 0) break;
		if ( dst[x] != src[x] ) {
			unsigned long * l_src = (unsigned long*) ((unsigned long) &src[x] & -8);
			unsigned long * l_dst = (unsigned long*) ((unsigned long) &dst[x] & -8);

			printf("error\t");
			printf("dst %p src %p n %4d\n", dst, src, len);

			printf("%d: %x %x\n", x, dst[x], src[x]);

			printf("%d: %16x %16x\n", x, *l_dst, *l_src);
//			exit(0);
//			break;
		}
	}

  munmap(shmaddr, 16384*2);

      } // for (j=0 ...
}

Re: [Linux-ia64] IA64 strncpy in 2.2.4-30 - bug and patch

[Andreas Schwab]

"Chen, Kenneth W" <kenneth.w.chen@intel.com> writes:

|> We found more bugs in the code, and here is an updated patch to fix them.
|> Note this patch supercedes the one I sent yesterday.

Could you please produce a patch against the version in glibc CVS?

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [Linux-ia64] IA64 strncpy in 2.2.4-30 - bug and patch

[Reese Faucette]

> We found more bugs in the code, and here is an updated patch to fix them.
> Note this patch supercedes the one I sent yesterday.

Your test programs and mine run fine with the patches.
thanks!
-reese

Re: [Linux-ia64] IA64 strncpy in 2.2.4-30 - bug and patch

[Randolph Chung]

In reference to a message from Chen, Kenneth W, dated Nov 07:
> We found more bugs in the code, and here is an updated patch to fix them.
> Note this patch supercedes the one I sent yesterday.
> 
> Also attached is the test program we used to verify the fix.

Is there an updated version of this that applies against glibc cvs? Or
is the one Jakub/Roland checked in sufficient (it looks more like the first
version you posted)

thanks,
randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

RE: [Linux-ia64] IA64 strncpy in 2.2.4-30 - bug and patch

[Chen, Kenneth W]

[-- Attachment #1: Type: text/plain, Size: 1424 bytes --]

Here is the updated patch against current glibc cvs strncpy.S v1.9.

There are two hunks in this patch.  The first hunk initialize ar.ec, ec is not guaranteed to be zero upon function entry, although most likely it will be zero.  But such assumption is functionally broken.

The second hunk fixes the seg fault in recovery 4 section.  The case of seg fault is that when src length ends exactly at a page boundary, and there are no mapping after that page.  In recovery 4, it is trying to access an invalid address due to earlier loop that src pointer got post incremented pass beyond current page.

- Ken


-----Original Message-----
From: Randolph Chung [mailto:randolph@tausq.org]
Sent: Tuesday, November 19, 2002 8:19 AM
To: Chen, Kenneth W
Cc: bug-glibc@gnu.org; linux-ia64@linuxia64.org
Subject: Re: [Linux-ia64] IA64 strncpy in 2.2.4-30 - bug and patch


In reference to a message from Chen, Kenneth W, dated Nov 07:
> We found more bugs in the code, and here is an updated patch to fix them.
> Note this patch supercedes the one I sent yesterday.
> 
> Also attached is the test program we used to verify the fix.

Is there an updated version of this that applies against glibc cvs? Or
is the one Jakub/Roland checked in sufficient (it looks more like the first
version you posted)

thanks,
randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

[-- Attachment #2: strncpy.fix.cvs_v1.9 --]
[-- Type: application/octet-stream, Size: 574 bytes --]

--- strncpy.S.orig	Tue Nov 19 10:22:01 2002
+++ strncpy.S	Tue Nov 19 10:29:21 2002
@@ -63,6 +63,7 @@
 	mov	saved_pr = pr           // save the predicate registers
 	.save ar.lc, saved_lc
 	mov 	saved_lc = ar.lc	// save the loop counter
+	mov	ar.ec = 0
 	.body
 	cmp.geu p6, p5 = 24, in2
 (p6)	br.cond.spnt .short_len
@@ -221,7 +222,9 @@
 	ld8	r[MEMLAT] = [tmp]
 	br.cond.sptk .back3
 .recovery4:
+	cmp.eq	p8,p9=0,len
 	add	tmp = -MEMLAT * 8, src ;;
-	ld8	r[MEMLAT - 1] = [tmp]
+(p8)	mov	r[MEMLAT - 1] = r0
+(p9)	ld8	r[MEMLAT - 1] = [tmp]
 	br.cond.sptk .back4
 END(strncpy)