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

Reese Faucette

author

2002-11-06 00:39 UTC #0 lore raw

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

strncpy.S +2 -2

@@ -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)

Chen, Kenneth W

2002-11-06 00:43 UTC #1 lore raw

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

-----Original Message-----
From: Reese Faucette [mailto:[email protected]]
Sent: Tuesday, November 05, 2002 4:39 PM
To: [email protected]; [email protected]
Cc: Akyil, Levent; [email protected]
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

Chen, Kenneth W

2002-11-07 02:56 UTC #2 lore raw

In reply to Reese Faucette

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; [email protected]; [email protected]
Cc: Akyil, Levent; [email protected]
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:[email protected]]
Sent: Tuesday, November 05, 2002 4:39 PM
To: [email protected]; [email protected]
Cc: Akyil, Levent; [email protected]
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
[email protected]
http://lists.linuxia64.org/lists/listinfo/linux-ia64

strncpy.S renamed from strncpy.S.orig +12 -4

@@ -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)

Chen, Kenneth W

2002-11-07 19:10 UTC #3 lore raw

In reply to 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.

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; [email protected]; [email protected]
Cc: Akyil, Levent; [email protected]; 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; [email protected]; [email protected]
Cc: Akyil, Levent; [email protected]
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:[email protected]]
Sent: Tuesday, November 05, 2002 4:39 PM
To: [email protected]; [email protected]
Cc: Akyil, Levent; [email protected]
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
[email protected]
http://lists.linuxia64.org/lists/listinfo/linux-ia64

strncpy.S renamed from strncpy.S.orig +12 -4

@@ -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)

Andreas Schwab

2002-11-08 11:06 UTC #4 lore raw

In reply to Reese Faucette

"Chen, Kenneth W" <[email protected]> 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.

Signature · 4 lines

Andreas Schwab, SuSE Labs, [email protected]
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."

Reese Faucette

author

2002-11-08 11:31 UTC #5 lore raw

In reply to Reese Faucette

Quoted 2 lines

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

Randolph Chung

2002-11-19 16:19 UTC #6 lore raw

In reply to Reese Faucette

In reference to a message from Chen, Kenneth W, dated Nov 07:

Quoted 4 lines

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

Signature · 3 lines

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

Chen, Kenneth W

2002-11-19 18:35 UTC #7 lore raw

In reply to Reese Faucette

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:[email protected]]
Sent: Tuesday, November 19, 2002 8:19 AM
To: Chen, Kenneth W
Cc: [email protected]; [email protected]
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:

Quoted 4 lines

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

Signature · 3 lines

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