Date: Sat, 16 Sep 2023 04:23:32 GMT From: Robert Clausecker <fuz@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 953b93cf24d8 - main - lib/libc/amd64/string/memcmp.S: harden against phony buffer lengths Message-ID: <202309160423.38G4NW9A019458@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by fuz: URL: https://cgit.FreeBSD.org/src/commit/?id=953b93cf24d8871c62416c9bcfca935f1f1853b6 commit 953b93cf24d8871c62416c9bcfca935f1f1853b6 Author: Robert Clausecker <fuz@FreeBSD.org> AuthorDate: 2023-09-14 05:19:01 +0000 Commit: Robert Clausecker <fuz@FreeBSD.org> CommitDate: 2023-09-16 04:20:32 +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 --- 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?202309160423.38G4NW9A019458>