⬢ DragonFlyBSD Kernel Audit
DF-5700 / fix.diff
← back to finding ↓ download raw
--- a/sys/libkern/bcmp.c
+++ b/sys/libkern/bcmp.c
@@ -41,103 +41,46 @@
 typedef	unsigned long		ul;
 typedef const unsigned long	*culp;
 
+/*
+ * bcmp -- vax cmpc3 instruction semantics: returns 0 iff the first
+ * `length' bytes compare equal, non-zero otherwise.
+ *
+ * This implementation never loads outside [b1, b1+length) or
+ * [b2, b2+length):  byte loads are used until BOTH pointers are
+ * longword-aligned (so the streaming loads always have at least
+ * sizeof(long) valid bytes remaining), full longwords are compared in
+ * the loop, and the remaining 0..sizeof(long)-1 tail bytes are
+ * compared byte-wise.  This removes the by-design longword over-read
+ * of the previous revision (which relied on aligned loads being
+ * page-contained, an assumption that only holds as long as every
+ * caller's buffer lives inside a larger protected region), and the
+ * size_t -> int truncation of `length' which made lengths >= 2^31
+ * compare the wrong number of bytes (and return a false "equal" for
+ * lengths that are a multiple of 2^32).
+ */
 int
 bcmp(const void *b1, const void *b2, size_t length)
 {
-#if BYTE_ORDER == LITTLE_ENDIAN
-	/*
-	 * The following code is endian specific.  Changing it from
-	 * little-endian to big-endian is fairly trivial, but making
-	 * it do both is more difficult.
-	 *
-	 * Note that this code will reference the entire longword which
-	 * includes the final byte to compare.  I don't believe this is
-	 * a problem since AFAIK, objects are not protected at smaller
-	 * than longword boundaries.
-	 */
-	int	shl, shr, len = length;
+	size_t	len = length;
 	ustring	p1 = b1, p2 = b2;
-	ul	va, vb;
 
 	if (len == 0)
 		return (0);
 
-	/*
-	 * align p1 to a longword boundary
-	 */
-	while ((long)p1 & (sizeof(long) - 1)) {
+	/* align BOTH pointers to a longword boundary */
+	while (((long)p1 | (long)p2) & (sizeof(long) - 1)) {
 		if (*p1++ != *p2++)
 			return (1);
 		if (--len <= 0)
 			return (0);
 	}
 
-	/*
-	 * align p2 to longword boundary and calculate the shift required to
-	 * align p1 and p2
-	 */
-	shr = (long)p2 & (sizeof(long) - 1);
-	if (shr != 0) {
-		p2 -= shr;			/* p2 now longword aligned */
-		shr <<= 3;			/* offset in bits */
-		shl = (sizeof(long) << 3) - shr;
-
-		va = *(culp)p2;
-		p2 += sizeof(long);
-
-		while ((len -= sizeof(long)) >= 0) {
-			vb = *(culp)p2;
-			p2 += sizeof(long);
-			if (*(culp)p1 != (va >> shr | vb << shl))
-				return (1);
-			p1 += sizeof(long);
-			va = vb;
-		}
-		/*
-		 * At this point, len is between -sizeof(long) and -1,
-		 * representing 0 .. sizeof(long)-1 bytes remaining.
-		 */
-		if (!(len += sizeof(long)))
-			return (0);
-
-		len <<= 3;		/* remaining length in bits */
-		/*
-		 * The following is similar to the `if' condition
-		 * inside the above while loop.  The ?: is necessary
-		 * to avoid accessing the longword after the longword
-		 * containing the last byte to be compared.
-		 */
-		return ((((va >> shr | ((shl < len) ? *(culp)p2 << shl : 0)) ^
-		    *(culp)p1) & ((1L << len) - 1)) != 0);
-	} else {
-		/* p1 and p2 have common alignment so no shifting needed */
-		while ((len -= sizeof(long)) >= 0) {
-			if (*(culp)p1 != *(culp)p2)
-				return (1);
-			p1 += sizeof(long);
-			p2 += sizeof(long);
-		}
-
-		/*
-		 * At this point, len is between -sizeof(long) and -1,
-		 * representing 0 .. sizeof(long)-1 bytes remaining.
-		 */
-		if (!(len += sizeof(long)))
-			return (0);
-
-		return (((*(culp)p1 ^ *(culp)p2)
-			 & ((1L << (len << 3)) - 1)) != 0);
-	}
-#else
-	char *p1, *p2;
-
-	if (length == 0)
-		return(0);
-	p1 = (char *)b1;
-	p2 = (char *)b2;
-	do
-		if (*p1++ != *p2++)
-			break;
-	while (--length);
-	return(length);
-#endif
+	/* full longwords: at least sizeof(long) bytes remain on both sides */
+	while (len >= sizeof(long)) {
+		if (*(culp)p1 != *(culp)p2)
+			return (1);
+		p1 += sizeof(long);
+		p2 += sizeof(long);
+		len -= sizeof(long);
+	}
+
+	/* byte-wise tail: 0 .. sizeof(long)-1 bytes remaining */
+	while (len--) {
+		if (*p1++ != *p2++)
+			return (1);
+	}
+	return (0);
 }