Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 23 Sep 2023 18:22:35 GMT
From:      Robert Clausecker <fuz@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 0666c6fc0322 - stable/14 - lib/libc/amd64/string/memcmp.S: harden against phony buffer lengths
Message-ID:  <202309231822.38NIMZlO004232@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/14 has been updated by fuz:

URL: https://cgit.FreeBSD.org/src/commit/?id=0666c6fc0322e0a4912288cd8b02213d52b2a9ce

commit 0666c6fc0322e0a4912288cd8b02213d52b2a9ce
Author:     Robert Clausecker <fuz@FreeBSD.org>
AuthorDate: 2023-09-14 05:19:01 +0000
Commit:     Robert Clausecker <fuz@FreeBSD.org>
CommitDate: 2023-09-23 18:21:42 +0000

    lib/libc/amd64/string/memcmp.S: harden against phony buffer lengths
    
    When memcmp(a, b, len) (or equally, bcmp) is called with a phony length
    such that a + len < a, the code would malfunction and not compare the
    two buffers correctly.  While such arguments are illegal (buffers do not
    wrap around the end of the address space), it is neverthless conceivable
    that people try things like memcmp(a, b, SIZE_MAX) to compare a and b
    until the first mismatch, in the knowledge that such a mismatch exists,
    expecting memcmp() to stop comparing somewhere around the mismatch.
    While memcmp() is usually written to confirm to this assumption, no
    version of ISO/IEC 9899 guarantees this behaviour (in contrast to
    memchr() for which it is).
    
    Neverthless it appears sensible to at least not grossly misbehave on
    phony lengths.  This change hardens memcmp() against this case by
    comparing at least until the end of the address space if a + len
    overflows a 64 bit integer.
    
    Sponsored by:   The FreeBSD Foundation
    Approved by:    mjg (blanket, via IRC)
    See also:       b2618b651b28fd29e62a4e285f5be09ea30a85d4
    MFC after:      1 week
    
    (cherry picked from commit 953b93cf24d8871c62416c9bcfca935f1f1853b6)
---
 lib/libc/amd64/string/memcmp.S | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/lib/libc/amd64/string/memcmp.S b/lib/libc/amd64/string/memcmp.S
index d192229677b3..dc8bcff73cb9 100644
--- a/lib/libc/amd64/string/memcmp.S
+++ b/lib/libc/amd64/string/memcmp.S
@@ -328,13 +328,28 @@ ARCHENTRY(memcmp, baseline)
 	movdqu		16(%rsi, %rdi, 1), %xmm1
 	pcmpeqb		16(%rdi), %xmm1		# compare second half of this iteration
 	add		%rcx, %rdx		# pointer to last byte in buffer
-	pcmpeqb		%xmm2, %xmm0
+	jc		.Loverflow		# did this overflow?
+0:	pcmpeqb		%xmm2, %xmm0
 	pmovmskb	%xmm0, %eax
 	xor		$0xffff, %eax		# any mismatch?
 	jne		.Lmismatch_head
 	add		$64, %rdi		# advance to next iteration
 	jmp		1f			# and get going with the loop
 
+	/*
+	 * If we got here, a buffer length was passed to memcmp(a, b, len)
+	 * such that a + len < a.  While this sort of usage is illegal,
+	 * it is plausible that a caller tries to do something like
+	 * memcmp(a, b, SIZE_MAX) if a and b are known to differ, intending
+	 * for memcmp() to stop comparing at the first mismatch.  This
+	 * behaviour is not guaranteed by any version of ISO/IEC 9899,
+	 * but usually works out in practice.  Let's try to make this
+	 * case work by comparing until the end of the address space.
+	 */
+.Loverflow:
+	mov		$-1, %rdx		# compare until the end of memory
+	jmp		0b
+
 	/* process buffer 32 bytes at a time */
 	ALIGN_TEXT
 0:	movdqu		-32(%rsi, %rdi, 1), %xmm0



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202309231822.38NIMZlO004232>