[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Alpha strncpy bug



Well, this was an interesting exposure to Alpha assembly.... a bug was
reported to Debian where strncpy would read past the end of the page on
Alpha systems, in some circumstances.  I tracked it down, and I extended
stratcliff.c to catch some more possible strncpy bugs.  Some caveats:

  - This is not the most efficient possible fix, I'd wager... it's a little
    sick.  I know nothing at all about Alpha scheduling so I assume I've
    destroyed the carefully handcrafted scheduling properties of this
    routine.
  - The copy in alphaev6/stxncpy.S probably needs similar fixes.
  - I haven't run the improved tests on any other architecture, so I don't
    know if others will have similar problems.

I'd appreciate it if someone more fluent in Alpha than I am would comment on
this, since I'm not qualified.

--- string/stratcliff.c	Sun Sep  8 14:46:38 2002
+++ string/stratcliff.c	Sun Sep  8 14:46:55 2002
@@ -192,7 +192,26 @@
 	    }
         }
 
-      /* strncpy test */
+      /* strncpy tests */
+
+      adr[size-1] = 'T';
+      for (outer = size - 1; outer >= MAX (0, size - 128); --outer)
+        {
+	  size_t len;
+
+	  for (len = 0; len < size - outer; ++len)
+	    {
+	      if (strncpy (dest, &adr[outer], len) != dest
+		  || memcmp (dest, &adr[outer], len) != 0)
+		{
+		  printf ("outer strncpy flunked for outer = %d, len = %Zd\n",
+			  outer, len);
+		  result = 1;
+		}
+	    }
+        }
+      adr[size-1] = '\0';
+
       for (outer = size - 1; outer >= MAX (0, size - 128); --outer)
         {
           for (inner = MAX (outer, size - 64); inner < size; ++inner)
@@ -210,6 +229,16 @@
 			  && strlen (dest) != (inner - outer)))
 		    {
 		      printf ("strncpy flunked for outer = %d, inner = %d, len = %Zd\n",
+			      outer, inner, len);
+		      result = 1;
+		    }
+		  if (strncpy (dest + 1, &adr[outer], len) != dest + 1
+		      || memcmp (dest + 1, &adr[outer],
+				 MIN (inner - outer, len)) != 0
+		      || (inner - outer < len
+			  && strlen (dest + 1) != (inner - outer)))
+		    {
+		      printf ("strncpy+1 flunked for outer = %d, inner = %d, len = %Zd\n",
 			      outer, inner, len);
 		      result = 1;
 		    }
--- sysdeps/alpha/stxncpy.S	Sun Sep  8 13:35:38 2002
+++ sysdeps/alpha/stxncpy.S	Sun Sep  8 14:46:13 2002
@@ -194,10 +194,23 @@
 	subq	a2, 1, a2		# e0    :
 	bne	t7, $u_late_head_exit	# .. e1 :
 
+	/* If the count is now zero, we may have read the last word we need
+	   already.  Avoid reading it, in case it would cause a SEGV.
+	   The condition we use is if the (adjusted) source misalignment
+	   plus the (adjusted) count misalignment are greater than a
+	   quadword.  */
+	bne a2, 1f
+	and a1, 7, t6
+	sll t10, t6, t6
+	addq zero, 1, t8
+	sll t8, 7, t8
+	cmplt t6, t8, t8
+	bne t8, $u_late_head_exit
+
 	/* Finally, we've got all the stupid leading edge cases taken care
 	   of and we can set up to enter the main loop.  */
 
-	extql	t2, a1, t1	# e0    : position hi-bits of lo word
+1:	extql	t2, a1, t1	# e0    : position hi-bits of lo word
 	ldq_u	t2, 8(a1)	# .. e1 : read next high-order source word
 	addq	a1, 8, a1	# e0    :
 	cmpbge	zero, t2, t7	# e1 (stall)
@@ -224,12 +237,12 @@
 	extql	t2, a1, t3	# e0    : extract low bits for next time
 	addq	a0, 8, a0	# .. e1 :
 	or	t0, t1, t0	# e0    : current dst word now complete
+	subq	a2, 1, a2	# e0    : [drow] scheduler death
+	beq	a2, $u_loopend
 	ldq_u	t2, 0(a1)	# .. e1 : load high word for next time
 	stq_u	t0, -8(a0)	# e0    : save the current word
 	mov	t3, t1		# .. e1 :
-	subq	a2, 1, a2	# e0    :
 	cmpbge	zero, t2, t7	# .. e1 : test new word for eos
-	beq	a2, $u_eoc	# e1    :
 	beq	t7, $u_loop	# e1    :
 
 	/* We've found a zero somewhere in the source word we just read.
@@ -287,6 +300,21 @@
 $u_eocfin:			# end-of-count, final word
 	or	t10, t7, t7
 	br	$u_final
+
+$u_loopend:
+	/* Should we load the last word?  As above.  */
+	and a1, 7, t6
+	sll t10, t6, t6
+	addq zero, 1, t8
+	sll t8, 7, t8
+	cmplt t6, t8, t8
+	bne t8, $u_eoc
+
+	ldq_u	t2, 0(a1)
+	stq_u	t0, -8(a0)
+	mov	t3, t1
+	cmpbge	zero, t2, t7
+	br	$u_eoc
 
 	/* Unaligned copy entry point.  */
 	.align 3


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer



Reply to: